JS: add joined paths as a sink to js/shell-command-constructed-from-input #5606
Conversation
erik-krogh
added
the
Awaiting evaluation
erik-krogh
removed
the
Awaiting evaluation
| joinCall = isExecutedAsShellCommand(DataFlow::TypeBackTracker::end(), sys) | ||
| } | ||
|
|
||
| override string getSinkType() { result = "Uncontrolled path" } |
There was a problem hiding this comment.
Minor issue:
I agree with the intent, but I think this string results in a confusing error message.
I fear the user will treat this as a false positive from js/path-injection since it is not clear from the message that the problem with the path is that it may be treated as more than just a file path.
There was a problem hiding this comment.
The full alert message with the current implementation is:
"Uncontrolled path based on library input is later used in shell command."
I agree that the mix of paths and shellscript terminology can be confusing, but I don't have a better idea right now.
Maybe calling it a "Path concatenation" could work better?
There was a problem hiding this comment.
Maybe calling it a "Path concatenation" could work better?
Agreed. That also mirrors the "String concatenation" sink type above.
erik-krogh
added
JS:changes-sources-or-sinks
Gets a TP/TN for CVE-2021-23352
Evaluation was uneventful.