JS: Modeling of ShellJS functions
#19422
Open
+67
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…Injection modules to use ThreatModelSource
There was a problem hiding this comment.
Pull Request Overview
This PR extends CodeQL’s JavaScript analysis to model additional ShellJS and async-shelljs functionality, specifically command location (which), generic command execution (cmd), asynchronous execution (asyncExec), and environment-variable sources (env).
- Added new tests and expected results for
shelljs.which,shelljs.cmd,async-shelljs.asyncExec, andshelljs.envin security query tests and library-tests. - Updated ShellJS QL framework (
ShellJS.qllandshelljs.model.yml) to include support for the new functions and to tagshelljs.envas an environment source. - Refactored environment-source handling in dataflow customization modules to use
ThreatModelSourcetagging.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js | Added test2 using shelljs.env to trigger indirect command injection alerts |
| javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected | Updated expected alerts for the new shelljs.env cases |
| javascript/ql/test/library-tests/frameworks/Shelljs/tst.js | Extended library tests with shelljs.cmd, shelljs.which, and asyncExec |
| javascript/ql/test/library-tests/frameworks/Shelljs/ShellJS.expected | Added expected flow entries for the new ShellJS calls |
| javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql | Switched to threat-model tagging for process.env source |
| javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll | Refactored ProcessEnvAsSource and envObject to use ThreatModelSource |
| javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll | Updated ProcessEnvSource to apply threat-model tagging |
| javascript/ql/lib/semmle/javascript/frameworks/ShellJS.qll | Added which, cmd, asyncExec to ShellJS model and adjusted call handling |
| javascript/ql/lib/ext/shelljs.model.yml | Declared shelljs.Member[env] as an environment source |
| javascript/ql/lib/change-notes/2025-04-30-shelljs.md | Documented the minor-analysis changes for new ShellJS/async-shelljs support |
Comments suppressed due to low confidence (3)
javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll:32
- The constructor for
ProcessEnvAsSourceno longer initializesthisto theprocess.envproperty read, so it won’t actually captureprocess.envas a source. You should restorethis = NodeJSLib::process().getAPropertyRead("env")and then apply the threat-model tag.
ProcessEnvAsSource() { this.(ThreatModelSource).getThreatModel() = "environment" }
javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll:174
- This constructor only applies the threat-model tag but never sets
thistoNodeJSLib::process().getAPropertyRead("env"). Without the property-read node, the source won’t be detected. Reintroduce the property-read assignment before tagging.
ProcessEnvSource() { this.(ThreatModelSource).getThreatModel() = "environment" }
javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql:40
- The original assignment
env = NodeJSLib::process().getAPropertyRead("env")was removed. Nowenvis never bound to the property-read node. You need to both assign the property-read and then tag it withThreatModelSource.
env.(ThreatModelSource).getThreatModel() = "environment" and
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added support for additional functions in the
shelljsandasync-shelljslibraries:which: Command location functionalitycmd: Command execution wrapperasyncExec: Asynchronous command executionenv: Environment variable handling