Skip to content

Commit 85959ea

Browse files
committed
Expand log injection sanitizer guards to non-annotation regex matches
1 parent a757178 commit 85959ea

File tree

1 file changed

+28
-17
lines changed

1 file changed

+28
-17
lines changed

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

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,24 +105,35 @@ private predicate logInjectionGuard(Guard g, Expr e, boolean branch) {
105105
or
106106
exists(RegexMatch rm, CompileTimeConstantExpr target |
107107
rm = g and
108+
not rm instanceof Annotation and
108109
target = rm.getRegex() and
109-
e = rm.getString()
110+
e = rm.getASanitizedExpr()
110111
|
111-
// Allow anything except line breaks
112-
(
113-
not target.getStringValue().matches("%[^%]%") and
114-
not target.getStringValue().matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
115-
or
116-
target.getStringValue().matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%")
117-
) and
118-
branch = true
119-
or
120-
// Disallow line breaks
121-
(
122-
not target.getStringValue().matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%") and
123-
// Assuming a regex containing line breaks is correctly matching line breaks in a string
124-
target.getStringValue().matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
125-
) and
126-
branch = false
112+
regexPreventsLogInjection(target.getStringValue(), branch)
127113
)
128114
}
115+
116+
/**
117+
* Holds if `regex` matches against a pattern that allows anything except
118+
* line breaks when `branch` is `true`, or a pattern that matches line breaks
119+
* when `branch` is `false`.
120+
*/
121+
bindingset[regex]
122+
private predicate regexPreventsLogInjection(string regex, boolean branch) {
123+
// Allow anything except line breaks
124+
(
125+
not regex.matches("%[^%]%") and
126+
not regex.matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
127+
or
128+
regex.matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%")
129+
) and
130+
branch = true
131+
or
132+
// Disallow line breaks
133+
(
134+
not regex.matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%") and
135+
// Assuming a regex containing line breaks is correctly matching line breaks in a string
136+
regex.matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
137+
) and
138+
branch = false
139+
}

0 commit comments

Comments
 (0)