Skip to content

Commit bb911bb

Browse files
erik-kroghesbena
andauthored
Apply suggestions from code review
Co-Authored-By: Esben Sparre Andreasen <esbena@github.com>
1 parent c83c27c commit bb911bb

File tree

1 file changed

+4
-3
lines changed

1 file changed

+4
-3
lines changed

javascript/ql/src/semmle/javascript/security/UselessUseOfCat.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ private class CommandCall extends DataFlow::InvokeNode {
7878
* Holds if the input `str` contains some character that might be interpreted in a non-trivial way by a shell.
7979
*/
8080
bindingset[str]
81-
predicate containsNonTrivialBashChar(string str) {
82-
exists(str.regexpFind("\\*|\\||>|<| |\\$|&|,|\\`| ", _, _))
81+
private predicate containsNonTrivialShellChar(string str) {
82+
exists(str.regexpFind("\\*|\\||>|<| |\\$|&|,|\\`| |;", _, _))
8383
}
8484

8585
/**
@@ -142,7 +142,7 @@ class UselessCat extends CommandCall {
142142
/**
143143
* Gets a string used to call `cat`.
144144
*/
145-
string getACatExecuteable() {
145+
private string getACatExecuteable() {
146146
result = "cat" or result = "/bin/cat"
147147
}
148148

@@ -173,6 +173,7 @@ module PrettyPrintCatCall {
173173
callback = "" and not exists(cat.getCallback())
174174
) and
175175
fileArg = createFileArgument(cat).trim() and
176+
// sanity check in case of surprising `toString` results, other uses of `containsNonTrivialBashChar` should ensure that this conjunct will hold most of the time
176177
not(containsNonTrivialBashChar(fileArg.regexpReplaceAll("\\$|\\`| ", ""))) // string concat might contain " ", template strings might contain "$" or `, and that is OK.
177178
|
178179
result =

0 commit comments

Comments
 (0)