Skip to content

Commit b633b8c

Browse files
owen-mcCopilot
owen-mc
and
Copilot
committed
Add string replacement and removal concepts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5c929a3 commit b633b8c

10 files changed

Lines changed: 322 additions & 137 deletions

File tree

‎java/ql/lib/semmle/code/java/Concepts.qll‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import java
1414
*/
1515
private module Frameworks {
1616
private import semmle.code.java.JDK
17+
private import semmle.code.java.frameworks.apache.Lang
18+
private import semmle.code.java.frameworks.guava.Guava
1719
private import semmle.code.java.frameworks.JavaxAnnotations
1820
}
1921

@@ -79,3 +81,107 @@ module RegexMatch {
7981
abstract string getName();
8082
}
8183
}
84+
85+
/**
86+
* An expression that replaces every occurrence of a target in a string.
87+
*
88+
* Extend this class to refine existing API models. If you want to model new APIs,
89+
* extend `StringReplacement::Range` instead.
90+
*/
91+
class StringReplacement extends Expr instanceof StringReplacement::Range {
92+
/** Gets the input string whose contents are replaced. */
93+
Expr getInputString() { result = super.getInputString() }
94+
95+
/** Gets the expression that identifies the content to replace. */
96+
Expr getMatchExpression() { result = super.getMatchExpression() }
97+
98+
/** Gets the replacement expression. */
99+
Expr getReplacement() { result = super.getReplacement() }
100+
101+
/** Holds if the match expression is interpreted as a regular expression. */
102+
predicate matchExpressionIsRegex() { super.matchExpressionIsRegex() }
103+
104+
/** Holds if each character in the match expression is matched independently. */
105+
predicate matchExpressionIsCharacterSet() { super.matchExpressionIsCharacterSet() }
106+
107+
/** Gets a descriptive name for this replacement operation. */
108+
string getName() { result = super.getName() }
109+
}
110+
111+
/** Provides classes for modeling string-replacement APIs. */
112+
module StringReplacement {
113+
/**
114+
* An expression that replaces every occurrence of a target in a string.
115+
*
116+
* Extend this class to model new APIs. If you want to refine existing API models,
117+
* extend `StringReplacement` instead.
118+
*/
119+
abstract class Range extends Expr {
120+
/** Gets the input string whose contents are replaced. */
121+
abstract Expr getInputString();
122+
123+
/** Gets the expression that identifies the content to replace. */
124+
abstract Expr getMatchExpression();
125+
126+
/** Gets the replacement expression. */
127+
abstract Expr getReplacement();
128+
129+
/** Holds if the match expression is interpreted as a regular expression. */
130+
predicate matchExpressionIsRegex() { none() }
131+
132+
/** Holds if each character in the match expression is matched independently. */
133+
predicate matchExpressionIsCharacterSet() { none() }
134+
135+
/** Gets a descriptive name for this replacement operation. */
136+
abstract string getName();
137+
}
138+
}
139+
140+
/**
141+
* An expression that removes every occurrence of a target from a string.
142+
*
143+
* Extend this class to refine existing API models. If you want to model new APIs,
144+
* extend `StringRemoval::Range` instead.
145+
*/
146+
class StringRemoval extends Expr instanceof StringRemoval::Range {
147+
/** Gets the input string whose contents are removed. */
148+
Expr getInputString() { result = super.getInputString() }
149+
150+
/** Gets the expression that identifies the content to remove. */
151+
Expr getMatchExpression() { result = super.getMatchExpression() }
152+
153+
/** Holds if the match expression is interpreted as a regular expression. */
154+
predicate matchExpressionIsRegex() { super.matchExpressionIsRegex() }
155+
156+
/** Holds if each character in the match expression is matched independently. */
157+
predicate matchExpressionIsCharacterSet() { super.matchExpressionIsCharacterSet() }
158+
159+
/** Gets a descriptive name for this removal operation. */
160+
string getName() { result = super.getName() }
161+
}
162+
163+
/** Provides classes for modeling string-removal APIs. */
164+
module StringRemoval {
165+
/**
166+
* An expression that removes every occurrence of a target from a string.
167+
*
168+
* Extend this class to model new APIs. If you want to refine existing API models,
169+
* extend `StringRemoval` instead.
170+
*/
171+
abstract class Range extends Expr {
172+
/** Gets the input string whose contents are removed. */
173+
abstract Expr getInputString();
174+
175+
/** Gets the expression that identifies the content to remove. */
176+
abstract Expr getMatchExpression();
177+
178+
/** Holds if the match expression is interpreted as a regular expression. */
179+
predicate matchExpressionIsRegex() { none() }
180+
181+
/** Holds if each character in the match expression is matched independently. */
182+
predicate matchExpressionIsCharacterSet() { none() }
183+
184+
/** Gets a descriptive name for this removal operation. */
185+
abstract string getName();
186+
}
187+
}

‎java/ql/lib/semmle/code/java/Expr.qll‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,17 @@ class CompileTimeConstantExpr extends Expr {
205205
result = this.(LiveLiteral).getValue().getStringValue()
206206
}
207207

208+
/**
209+
* Gets the string value of this expression if it is a `String` constant, or
210+
* the single-character string value if it is a `char` constant.
211+
*/
212+
string getStringOrCharValue() {
213+
result = this.getStringValue()
214+
or
215+
this.getType() instanceof CharacterType and
216+
result = this.getIntValue().toUnicode()
217+
}
218+
208219
/**
209220
* Gets the boolean value of this expression, where possible.
210221
*/

‎java/ql/lib/semmle/code/java/JDK.qll‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,23 +64,41 @@ class StringMatchesCall extends MethodCall, RegexMatch::Range {
6464
}
6565

6666
/** A call to the `java.lang.String.replaceAll` method. */
67-
class StringReplaceAllCall extends MethodCall {
67+
class StringReplaceAllCall extends MethodCall, StringReplacement::Range {
6868
StringReplaceAllCall() {
6969
exists(Method m | m = this.getMethod() |
7070
m.getDeclaringType() instanceof TypeString and
7171
m.hasName("replaceAll")
7272
)
7373
}
74+
75+
override Expr getInputString() { result = this.getQualifier() }
76+
77+
override Expr getMatchExpression() { result = this.getArgument(0) }
78+
79+
override Expr getReplacement() { result = this.getArgument(1) }
80+
81+
override predicate matchExpressionIsRegex() { any() }
82+
83+
override string getName() { result = "String.replaceAll" }
7484
}
7585

7686
/** A call to the `java.lang.String.replace` method. */
77-
class StringReplaceCall extends MethodCall {
87+
class StringReplaceCall extends MethodCall, StringReplacement::Range {
7888
StringReplaceCall() {
7989
exists(Method m | m = this.getMethod() |
8090
m.getDeclaringType() instanceof TypeString and
8191
m.hasName("replace")
8292
)
8393
}
94+
95+
override Expr getInputString() { result = this.getQualifier() }
96+
97+
override Expr getMatchExpression() { result = this.getArgument(0) }
98+
99+
override Expr getReplacement() { result = this.getArgument(1) }
100+
101+
override string getName() { result = "String.replace" }
84102
}
85103

86104
/**

‎java/ql/lib/semmle/code/java/frameworks/apache/Lang.qll‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,24 @@ class TypeApacheSystemUtils extends Class {
4747
this.hasQualifiedName(["org.apache.commons.lang", "org.apache.commons.lang3"], "SystemUtils")
4848
}
4949
}
50+
51+
/** A call to `StringUtils.replaceChars`. */
52+
private class ApacheStringUtilsReplaceCharsCall extends MethodCall, StringReplacement::Range {
53+
ApacheStringUtilsReplaceCharsCall() {
54+
this.getMethod()
55+
.hasQualifiedName(["org.apache.commons.lang", "org.apache.commons.lang3"], "StringUtils",
56+
"replaceChars")
57+
}
58+
59+
override Expr getInputString() { result = this.getArgument(0) }
60+
61+
override Expr getMatchExpression() { result = this.getArgument(1) }
62+
63+
override Expr getReplacement() { result = this.getArgument(2) }
64+
65+
override predicate matchExpressionIsCharacterSet() {
66+
this.getMethod().getParameterType(1) instanceof TypeString
67+
}
68+
69+
override string getName() { result = "StringUtils.replaceChars" }
70+
}

‎java/ql/lib/semmle/code/java/frameworks/guava/Guava.qll‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,56 @@ module;
66

77
import java
88
import Collections
9+
private import semmle.code.java.dataflow.DataFlow
10+
11+
private DataFlow::Node getALocalOriginNode(Expr expr) {
12+
DataFlow::localFlow(result, DataFlow::exprNode(expr)) and
13+
not DataFlow::localFlowStep(_, result)
14+
}
15+
16+
private Expr getUniqueLocalOrigin(Expr expr) {
17+
result = unique(DataFlow::Node src | src = getALocalOriginNode(expr)).asExpr()
18+
}
19+
20+
/** A call to `CharMatcher.anyOf`. */
21+
private class CharMatcherAnyOfCall extends MethodCall {
22+
CharMatcherAnyOfCall() {
23+
this.getMethod().hasQualifiedName("com.google.common.base", "CharMatcher", "anyOf")
24+
}
25+
}
26+
27+
/** A call to `CharMatcher.replaceFrom`. */
28+
private class CharMatcherReplaceFromCall extends MethodCall, StringReplacement::Range {
29+
CharMatcherReplaceFromCall() {
30+
this.getMethod().hasQualifiedName("com.google.common.base", "CharMatcher", "replaceFrom")
31+
}
32+
33+
override Expr getInputString() { result = this.getArgument(0) }
34+
35+
override Expr getMatchExpression() {
36+
result = getUniqueLocalOrigin(this.getQualifier()).(CharMatcherAnyOfCall).getArgument(0)
37+
}
38+
39+
override Expr getReplacement() { result = this.getArgument(1) }
40+
41+
override predicate matchExpressionIsCharacterSet() { any() }
42+
43+
override string getName() { result = "CharMatcher.replaceFrom" }
44+
}
45+
46+
/** A call to `CharMatcher.removeFrom`. */
47+
private class CharMatcherRemoveFromCall extends MethodCall, StringRemoval::Range {
48+
CharMatcherRemoveFromCall() {
49+
this.getMethod().hasQualifiedName("com.google.common.base", "CharMatcher", "removeFrom")
50+
}
51+
52+
override Expr getInputString() { result = this.getArgument(0) }
53+
54+
override Expr getMatchExpression() {
55+
result = getUniqueLocalOrigin(this.getQualifier()).(CharMatcherAnyOfCall).getArgument(0)
56+
}
57+
58+
override predicate matchExpressionIsCharacterSet() { any() }
59+
60+
override string getName() { result = "CharMatcher.removeFrom" }
61+
}

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

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -48,52 +48,70 @@ private class LineBreaksLogInjectionSanitizer extends LogInjectionSanitizer {
4848
}
4949
}
5050

51-
private predicate stringMethodCall(
52-
MethodCall mc, CompileTimeConstantExpr arg0, CompileTimeConstantExpr arg1
53-
) {
54-
mc.getMethod().getDeclaringType() instanceof TypeString and
55-
arg0 = mc.getArgument(0) and
56-
arg1 = mc.getArgument(1)
51+
bindingset[match]
52+
pragma[inline_late]
53+
private predicate stringOrCharValueMatches(CompileTimeConstantExpr const, string match) {
54+
const.getStringOrCharValue().matches(match)
5755
}
5856

59-
private predicate stringMethodArgument(CompileTimeConstantExpr arg) {
60-
stringMethodCall(_, arg, _) or stringMethodCall(_, _, arg)
57+
private predicate containsLineBreak(CompileTimeConstantExpr const) {
58+
stringOrCharValueMatches(const, ["%\n%", "%\r%"])
6159
}
6260

63-
bindingset[match]
64-
pragma[inline_late]
65-
private predicate stringMethodArgumentValueMatches(CompileTimeConstantExpr const, string match) {
66-
stringMethodArgument(const) and
67-
const.getStringValue().matches(match)
61+
private predicate containsBothLineBreaks(CompileTimeConstantExpr const) {
62+
stringOrCharValueMatches(const, "%\n%") and stringOrCharValueMatches(const, "%\r%")
63+
}
64+
65+
private predicate regexRemovesLineBreaks(CompileTimeConstantExpr target) {
66+
// Replace anything not in an allow list
67+
target.getStringValue().matches("[^%]") and
68+
not stringOrCharValueMatches(target, "%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
69+
or
70+
// Replace line breaks
71+
target.getStringValue() = ["\n", "\r", "\\n", "\\r", "\\R"]
72+
}
73+
74+
private predicate replacementRemovesLineBreaks(
75+
StringReplacement replacement, CompileTimeConstantExpr target
76+
) {
77+
target = replacement.getMatchExpression() and
78+
(
79+
replacement.matchExpressionIsCharacterSet() and containsBothLineBreaks(target)
80+
or
81+
replacement.matchExpressionIsRegex() and regexRemovesLineBreaks(target)
82+
or
83+
not replacement.matchExpressionIsCharacterSet() and
84+
not replacement.matchExpressionIsRegex() and
85+
target.getStringOrCharValue() = ["\n", "\r"]
86+
)
87+
}
88+
89+
private predicate removalRemovesLineBreaks(StringRemoval removal, CompileTimeConstantExpr target) {
90+
target = removal.getMatchExpression() and
91+
(
92+
removal.matchExpressionIsCharacterSet() and containsBothLineBreaks(target)
93+
or
94+
removal.matchExpressionIsRegex() and regexRemovesLineBreaks(target)
95+
or
96+
not removal.matchExpressionIsCharacterSet() and
97+
not removal.matchExpressionIsRegex() and
98+
target.getStringOrCharValue() = ["\n", "\r"]
99+
)
68100
}
69101

70102
/**
71103
* Holds if `e` is sanitized against log injection attacks by removing line
72104
* breaks from it.
73105
*/
74106
private predicate logInjectionSanitizer(Expr e) {
75-
exists(MethodCall mc, CompileTimeConstantExpr target, CompileTimeConstantExpr replacement |
76-
e = mc and
77-
stringMethodCall(mc, target, replacement) and
78-
not stringMethodArgumentValueMatches(replacement, ["%\n%", "%\r%"])
79-
|
80-
mc.getMethod().hasName("replace") and
81-
not replacement.getIntValue() = [10, 13] and
82-
(
83-
target.getIntValue() = [10, 13] or // 10 == '\n', 13 == '\r'
84-
target.getStringValue() = ["\n", "\r"]
85-
)
86-
or
87-
mc.getMethod().hasName("replaceAll") and
88-
(
89-
// Replace anything not in an allow list
90-
target.getStringValue().matches("[^%]") and
91-
not stringMethodArgumentValueMatches(target, "%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
92-
or
93-
// Replace line breaks
94-
target.getStringValue() = ["\n", "\r", "\\n", "\\r", "\\R"]
95-
)
107+
replacementRemovesLineBreaks(e, _) and
108+
exists(CompileTimeConstantExpr replacement |
109+
replacement = e.(StringReplacement).getReplacement() and
110+
exists(replacement.getStringOrCharValue()) and
111+
not containsLineBreak(replacement)
96112
)
113+
or
114+
removalRemovesLineBreaks(e, _)
97115
}
98116

99117
/**

0 commit comments

Comments
 (0)