Skip to content

Commit 44b0950

Browse files
authored
Merge pull request github#13408 from aschackmull/java/loginjection-perf
Java: Add more negation context to reduce string ops and improve perf.
2 parents e8b12ce + 5a2ac1b commit 44b0950

File tree

1 file changed

+22
-5
lines changed

1 file changed

+22
-5
lines changed

java/ql/lib/semmle/code/java/security/LogInjection.qll

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,33 @@ private class LineBreaksLogInjectionSanitizer extends LogInjectionSanitizer {
4646
}
4747
}
4848

49+
private predicate stringMethodAccess(
50+
MethodAccess ma, CompileTimeConstantExpr arg0, CompileTimeConstantExpr arg1
51+
) {
52+
ma.getMethod().getDeclaringType() instanceof TypeString and
53+
arg0 = ma.getArgument(0) and
54+
arg1 = ma.getArgument(1)
55+
}
56+
57+
private predicate stringMethodArgument(CompileTimeConstantExpr arg) {
58+
stringMethodAccess(_, arg, _) or stringMethodAccess(_, _, arg)
59+
}
60+
61+
bindingset[match]
62+
pragma[inline_late]
63+
private predicate stringMethodArgumentValueMatches(CompileTimeConstantExpr const, string match) {
64+
stringMethodArgument(const) and
65+
const.getStringValue().matches(match)
66+
}
67+
4968
/**
5069
* Holds if the return value of `ma` is sanitized against log injection attacks
5170
* by removing line breaks from it.
5271
*/
5372
private predicate logInjectionSanitizer(MethodAccess ma) {
5473
exists(CompileTimeConstantExpr target, CompileTimeConstantExpr replacement |
55-
ma.getMethod().getDeclaringType() instanceof TypeString and
56-
target = ma.getArgument(0) and
57-
replacement = ma.getArgument(1) and
58-
not replacement.getStringValue().matches(["%\n%", "%\r%"])
74+
stringMethodAccess(ma, target, replacement) and
75+
not stringMethodArgumentValueMatches(replacement, ["%\n%", "%\r%"])
5976
|
6077
ma.getMethod().hasName("replace") and
6178
not replacement.getIntValue() = [10, 13] and
@@ -68,7 +85,7 @@ private predicate logInjectionSanitizer(MethodAccess ma) {
6885
(
6986
// Replace anything not in an allow list
7087
target.getStringValue().matches("[^%]") and
71-
not target.getStringValue().matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
88+
not stringMethodArgumentValueMatches(target, "%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
7289
or
7390
// Replace line breaks
7491
target.getStringValue() = ["\n", "\r", "\\n", "\\r", "\\R"]

0 commit comments

Comments
 (0)