codeql
codeql copied to clipboard
github
JS: provide command execution sinks for execa package
Execa package before version 5 has already been modeled but newer versions up to 8 have many new APIs that I've implemented now.
@erik-krogh the SecondOrderCommandInjectionQuery doesn't work for me here. please look at the test for examples, I already put comments on which lines are vulnerable to second-order command injection.
@erik-krogh sorry for the many mistakes on this PR. I tried to solve all of your review suggestions.
@erik-krogh sorry for the many mistakes on this PR. I tried to solve all of your review suggestions.
That's OK. We set a high bar here for testing and code quality, especially when you try to get something merged into our standard query set.
(Take a look at the experimental/ folder if you want to have your PR merged faster, but then you don't have as much impact).
In other news.
I just opened a PR to recognize tagged template literals as a DataFlow::CallNode: https://github.com/github/codeql/pull/14405
I suggest you take a look at that PR and incorporate it into your PR after my PR has been merged. There is no rush, so just wait until my PR has been merged.
In other news. I just opened a PR to recognize tagged template literals as a
DataFlow::CallNode: #14405
https://github.com/github/codeql/pull/14405 has been merged, so you can incorporate that into this PR.
@erik-krogh it is great that I can reach each part of the following example with the API graph:
$({shell: false}).sync`${cmd} ${arg} ${arg} ${arg2}`; // NOT OK
I think there is one problem. if we have :
$({shell: false}).sync`ssh ${arg} ${arg} ${arg2}`; // NOT OK
then I can't specify getArgumentList and getACommandArgument because I can't determine the tag literal string value to check whether there is a hardcoded command or not.
please look at the test examples and run the ExecaScript class predicates for execa.js I tried to fully support the test cases.
Also, it seems that when I'm using a predicate like isExecaShellEnable inside the isShellInterpreted predicate of ExecaScript, I can't reach other parameters.
I can't determine the tag literal string value to check whether there is a hardcoded command or not.
Well, you can, it's just a little tricky.
If you get the TaggedTemplateExpr (using e.g. node.asSink().asExpr().(TaggedTemplateExpr)), then you can get the template elements from that, and you can use those to determine whether the first argument encodes the command or not.
@erik-krogh should I move all of these PR changes to the experimental directory too?
@erik-krogh should I move all of these PR changes to the experimental directory too?
That's up to you.
But this PR can be merged much faster if you do.
@erik-krogh I moved this PR to experimental dir.
Looks OK. I'll give this a final review when you mark this PR as ready for review.
@erik-krogh this PR also is not marked as ready for review but you reviewed this before, can I mark this as Ready for review too?
The branch was getting old. I did a merge with main which should hopefully also trigger the CI.
Edit: Another merge with main. A CI failure was confusing, so I'll see if that gets fixed on its own.
Edit Edit: It did.