JS: track shell:true more in js/shell-command-constructed-from-input #11858
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| exists(API::Node node | | ||
| node.asSink() = sys.getOptionsArg() and | ||
| node.getMember("shell").getAValueReachingSink().mayHaveBooleanValue(true) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Why is the old bit not covered by the new bit (so that we could have only the new bit)?
Do you happen to have a concrete piece of code that is matched by the old and not the new?
There was a problem hiding this comment.
We are not guarantee that all SystemCommandExecution have a corresponding API::Node.
I think there are API::Node for all of them right now, but if we model more SystemCommandExecution they might not all have API::Nodes.
There was a problem hiding this comment.
Right, I can accept that. I might add a comment before line 162 saying something like "In case sys.getOptionsArg is not a sink for any API node".
| DataFlow::Node getAValueReachingSink() { | ||
| result = Impl::trackDefNode(this.asSink()) | ||
| or | ||
| // in case `asSink()` is not a `SourceNode`. | ||
| result = this.asSink() | ||
| } |
There was a problem hiding this comment.
If asSink() is not a SourceNode, you should simply get a local source for it, is that not good enough? I could se it failing, if rhs(_, _, asSink()) does not hold at all, but then that predicate should perhaps be extended? What is the concrete value that prompted this?
There was a problem hiding this comment.
If asSink() is not a
SourceNode, you should simply get a local source for it
The problem is that .getALocalSource() has no result.
A boolean literal is not a SourceNode, and .getALocalSource() has no result.
I think SourceNode in Python is a bit different. I suspect that you are guaranteed that .getALocalSource() has a result?
The motivation was a boolean literal being written to a property.
Now that I think about it: getAValueReachingSink adds nothing when we are tracking booleans, I would need a small-step type-backtracker (because that doesn't depend on SourceNode).
There was a problem hiding this comment.
Indeed, in Python there will always be a local source. If nothing else, then the node itself.
CVE-2022-22984: TP
Evaluation was unevenful.