github-community-projects / github-community-projects/safe-settings

Code injection vulnerability in config validators

Open
#979 0 comments 1 reaction 2 assignees Claimed by @decyjphr View on GitHub
bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.