github-community-projects / github-community-projects/safe-settings
Code injection vulnerability in config validators
- Dominant language
- JavaScript
- Stars
- 921
- Forks
- 226
- Avg merge
- 18h 3m
- Merged PRs (30d)
- 14
Description
## Problem Description
Code injection vulnerability in the [safe-settings open source repo](https://github.com/github/safe-settings/blob/main-enterprise/lib/settings.js#L117-L123) configvalidators feature that allows admin repo contributors to execute arbitrary code on the safe-settings server.
A user with write access to the admin repo can add a malicious configvalidators entry to .github/settings.yml. When safe-settings processes this config, it passes the script field directly to new Function() - essentially eval(). This allows the attacker to access process.env, including the GitHub App's private key, and exfiltrate it. With the private key, they can impersonate the App and access all repos where it's installed.
### Vulnerable code path:
Code injection point: lib/settings.js lines 117-123 (https://github.com/github/safe-settings/blob/main/lib/settings.js#L117-L123)
Config merge: index.js line 28 (https://github.com/github/safe-settings/blob/main/index.js#L28) - admin repo can override server conf
### What is actually happening
### What is the expected behavior
configvalidators and overridevalidators are only supposed to be created from the `deployment-settings.yml` which is strictly sourced from the filesystem on the server where the app runs and so cannot be overwritten or modified by users of safe-settings. Only the admin who is hosting the app will have access to `deployment-settings.yml` . To ensure that these properties are not coming from other settings ymls, if these properties are present in other settings files they should be ignored/stripped.
## Context
### Vulnerable code path:
Code injection point: lib/settings.js lines 117-123 (https://github.com/github/safe-settings/blob/main/lib/settings.js#L117-L123)
Config merge: index.js line 28 (https://github.com/github/safe-settings/blob/main/index.js#L28) - admin repo can override server conf
Contributor guide
Assessment
This issue has not been assessed yet.