Skip to content

Commit ac0913c

Browse files
author
Esben Sparre Andreasen
committed
JS: add newline removal whitelist for js/incomplete-sanitization
1 parent bdbd00e commit ac0913c

File tree

2 files changed

+17
-2
lines changed

2 files changed

+17
-2
lines changed

javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,20 @@ predicate isDelimiterUnwrapper(
129129
)
130130
}
131131

132+
/*
133+
* Holds if `repl` is a standalone use of `String.prototype.replace` to remove a single newline.
134+
*/
135+
136+
predicate removesTrailingNewLine(DataFlow::MethodCallNode repl) {
137+
repl.getMethodName() = "replace" and
138+
repl.getArgument(0).mayHaveStringValue("\n") and
139+
repl.getArgument(1).mayHaveStringValue("") and
140+
not exists(DataFlow::MethodCallNode other | other.getMethodName() = "replace" |
141+
repl.getAMethodCall() = other or
142+
other.getAMethodCall() = repl
143+
)
144+
}
145+
132146
from MethodCallExpr repl, Expr old, string msg
133147
where
134148
repl.getMethodName() = "replace" and
@@ -153,7 +167,9 @@ where
153167
not DataFlow::valueNode(repl.getReceiver()) = DataFlow::valueNode(repl).getASuccessor+() and
154168
// dont' flag unwrapper
155169
not isDelimiterUnwrapper(repl.flow(), _) and
156-
not isDelimiterUnwrapper(_, repl.flow())
170+
not isDelimiterUnwrapper(_, repl.flow()) and
171+
// dont' flag the removal of trailing newlines
172+
not removesTrailingNewLine(repl.flow())
157173
or
158174
exists(RegExpLiteral rel |
159175
isBackslashEscape(repl, rel) and

javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteSanitization.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
| tst.js:140:2:140:27 | s.repla ... replace | This replaces only the first occurrence of /}/. |
2626
| tst.js:141:2:141:10 | s.replace | This replaces only the first occurrence of ']'. |
2727
| tst.js:141:2:141:27 | s.repla ... replace | This replaces only the first occurrence of '['. |
28-
| tst.js:146:2:146:68 | require ... replace | This replaces only the first occurrence of "\\n". |
2928
| tst.js:148:2:148:10 | x.replace | This replaces only the first occurrence of "\\n". |
3029
| tst.js:149:2:149:24 | x.repla ... replace | This replaces only the first occurrence of "\\n". |
3130
| tst.js:193:9:193:17 | s.replace | This replaces only the first occurrence of /'/. |

0 commit comments

Comments
 (0)