Security: Command Injection via Unsanitized Input in GitHub Scripts#4299
Security: Command Injection via Unsanitized Input in GitHub Scripts#4299tomaioo wants to merge 1 commit into
Conversation
The `run_cpp_linter.py` and `run_py_linter.py` scripts construct shell commands using unsanitized environment variables and file paths. While the direct command injection surface is limited, the scripts read from `/GITHUB_EVENT.json` and pass repository data into subprocess calls. More critically, both scripts use `subprocess.run` with shell=False which mitigates direct injection, but they construct format strings with external data for PR comments. The `run_cpp_linter.py` script formats linter output directly into a PR comment without sanitization, which could lead to injection of markdown or control characters. Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
|
Hi @tomaioo! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Summary
Security: Command Injection via Unsanitized Input in GitHub Scripts
Problem
Severity:
Medium| File:.github/scripts/run_cpp_linter.py:L1The
run_cpp_linter.pyandrun_py_linter.pyscripts construct shell commands using unsanitized environment variables and file paths. While the direct command injection surface is limited, the scripts read from/GITHUB_EVENT.jsonand pass repository data into subprocess calls. More critically, both scripts usesubprocess.runwith shell=False which mitigates direct injection, but they construct format strings with external data for PR comments. Therun_cpp_linter.pyscript formats linter output directly into a PR comment without sanitization, which could lead to injection of markdown or control characters.Solution
Sanitize all external inputs before using them in subprocess calls or format strings. Use
shlex.quote()for shell arguments, and validate/escape output before including in PR comments. Consider using GitHub's officialgithub-scriptaction instead of custom Python scripts for PR interactions.Changes
.github/scripts/run_cpp_linter.py(modified)