JS: add shell interpreted arrays as sinks for js/shell-command-constructed-from-input #5458
Conversation
erik-krogh
added
Awaiting evaluation
erik-krogh
removed
the
Awaiting evaluation
|
|
||
| override SystemCommandExecution getCommandExecution() { result = sys } | ||
|
|
||
| override DataFlow::Node getAlertLocation() { result = this } |
There was a problem hiding this comment.
Could we use this to make the alert appear at the actual spawn call instead of where the array is constructed?
There was a problem hiding this comment.
Yes we could.
But the alert message already includes a link to the spawn call.
I currently have the alert-location set to the location where the taint-tracking ends (or, close to in the case of string-concats).
I thought it could be confusing to have a path-query where the path doesn't end in the alert-location.
Changing the alert-location to the spawn call would just require changing sinkNode.getAlertLocation() to sinkNode.getCommandExecution() in the .ql file, and changing the message a bit.
But I think I prefer not doing that.
There was a problem hiding this comment.
Ok fair enough. It would have been nice for the path to end at the argument to spawn, but I can see that would require adding flow labels to do properly.
Recognizes the sink in CVE-2020-28494
Evaluation looks fine.