Skip to content

Commit fa5bd75

Browse files
committed
Expand log injection sanitizers to annotation regex matches
1 parent 85959ea commit fa5bd75

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,12 @@ private predicate stringMethodArgumentValueMatches(CompileTimeConstantExpr const
6464
}
6565

6666
/**
67-
* Holds if the return value of `ma` is sanitized against log injection attacks
68-
* by removing line breaks from it.
67+
* Holds if `e` is sanitized against log injection attacks by removing line
68+
* breaks from it.
6969
*/
70-
private predicate logInjectionSanitizer(MethodCall ma) {
71-
exists(CompileTimeConstantExpr target, CompileTimeConstantExpr replacement |
70+
private predicate logInjectionSanitizer(Expr e) {
71+
exists(MethodCall ma, CompileTimeConstantExpr target, CompileTimeConstantExpr replacement |
72+
e = ma and
7273
stringMethodCall(ma, target, replacement) and
7374
not stringMethodArgumentValueMatches(replacement, ["%\n%", "%\r%"])
7475
|
@@ -89,6 +90,13 @@ private predicate logInjectionSanitizer(MethodCall ma) {
8990
target.getStringValue() = ["\n", "\r", "\\n", "\\r", "\\R"]
9091
)
9192
)
93+
or
94+
exists(RegexMatch rm, CompileTimeConstantExpr target |
95+
rm instanceof Annotation and
96+
e = rm.getASanitizedExpr() and
97+
target = rm.getRegex() and
98+
regexPreventsLogInjection(target.getStringValue(), true)
99+
)
92100
}
93101

94102
/**

java/ql/test/query-tests/security/CWE-117/LogInjectionTest.expected

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@
4646
| LogInjectionTest.java:183:26:183:31 | source | LogInjectionTest.java:75:34:75:41 | source(...) : Object | LogInjectionTest.java:183:26:183:31 | source | This log entry depends on a $@. | LogInjectionTest.java:75:34:75:41 | source(...) | user-provided value |
4747
| LogInjectionTest.java:187:26:187:31 | source | LogInjectionTest.java:75:34:75:41 | source(...) : Object | LogInjectionTest.java:187:26:187:31 | source | This log entry depends on a $@. | LogInjectionTest.java:75:34:75:41 | source(...) | user-provided value |
4848
| LogInjectionTest.java:193:26:193:31 | source | LogInjectionTest.java:75:34:75:41 | source(...) : Object | LogInjectionTest.java:193:26:193:31 | source | This log entry depends on a $@. | LogInjectionTest.java:75:34:75:41 | source(...) | user-provided value |
49-
| LogInjectionTest.java:198:22:198:40 | validatedInputField | LogInjectionTest.java:198:22:198:40 | validatedInputField | LogInjectionTest.java:198:22:198:40 | validatedInputField | This log entry depends on a $@. | LogInjectionTest.java:198:22:198:40 | validatedInputField | user-provided value |
50-
| LogInjectionTest.java:199:22:199:37 | validatedInput(...) | LogInjectionTest.java:199:22:199:37 | validatedInput(...) | LogInjectionTest.java:199:22:199:37 | validatedInput(...) | This log entry depends on a $@. | LogInjectionTest.java:199:22:199:37 | validatedInput(...) | user-provided value |
5149
| LogInjectionTest.java:205:39:205:55 | (...)... | LogInjectionTest.java:205:48:205:55 | source(...) : Object | LogInjectionTest.java:205:39:205:55 | (...)... | This log entry depends on a $@. | LogInjectionTest.java:205:48:205:55 | source(...) | user-provided value |
5250
| LogInjectionTest.java:206:28:206:35 | source(...) | LogInjectionTest.java:206:28:206:35 | source(...) | LogInjectionTest.java:206:28:206:35 | source(...) | This log entry depends on a $@. | LogInjectionTest.java:206:28:206:35 | source(...) | user-provided value |
5351
| LogInjectionTest.java:207:28:207:35 | source(...) | LogInjectionTest.java:207:28:207:35 | source(...) | LogInjectionTest.java:207:28:207:35 | source(...) | This log entry depends on a $@. | LogInjectionTest.java:207:28:207:35 | source(...) | user-provided value |
@@ -4517,8 +4515,6 @@ nodes
45174515
| LogInjectionTest.java:183:26:183:31 | source | semmle.label | source |
45184516
| LogInjectionTest.java:187:26:187:31 | source | semmle.label | source |
45194517
| LogInjectionTest.java:193:26:193:31 | source | semmle.label | source |
4520-
| LogInjectionTest.java:198:22:198:40 | validatedInputField | semmle.label | validatedInputField |
4521-
| LogInjectionTest.java:199:22:199:37 | validatedInput(...) | semmle.label | validatedInput(...) |
45224518
| LogInjectionTest.java:205:39:205:55 | (...)... | semmle.label | (...)... |
45234519
| LogInjectionTest.java:205:48:205:55 | source(...) : Object | semmle.label | source(...) : Object |
45244520
| LogInjectionTest.java:206:28:206:35 | source(...) | semmle.label | source(...) |
@@ -8133,3 +8129,6 @@ nodes
81338129
| LogInjectionTest.java:2151:38:2151:54 | (...)... | semmle.label | (...)... |
81348130
| LogInjectionTest.java:2151:47:2151:54 | source(...) : Object | semmle.label | source(...) : Object |
81358131
subpaths
8132+
testFailures
8133+
| LogInjectionTest.java:198:44:198:63 | // $ SPURIOUS: Alert | Fixed spurious result: Alert |
8134+
| LogInjectionTest.java:199:41:199:60 | // $ SPURIOUS: Alert | Fixed spurious result: Alert |

0 commit comments

Comments
 (0)