Skip to content

Commit 662852b

Browse files
aschackmullaschackmull
authored
Merge pull request github#6859 from smowton/smowton/admin/factor-string-prefix
Java: Factor out string prefix logic
2 parents b975e12 + d46b897 commit 662852b

File tree

2 files changed

+183
-142
lines changed

2 files changed

+183
-142
lines changed

‎java/ql/lib/semmle/code/java/dataflow/StringPrefixes.qll

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/**
2+
* Provides classes and predicates for identifying expressions that may be appended to an interesting prefix.
3+
*
4+
* To use this library, extend the abstract class `InterestingPrefix` to have the library identify expressions that
5+
* may be appended to it, then check `InterestingPrefix.getAnAppendedExpression(Expr)` to get your results.
6+
*
7+
* For example, to identify expressions that may follow "foo:" in some string, we could define:
8+
*
9+
* ```
10+
* private class FooPrefix extends InterestingPrefix {
11+
* int offset;
12+
* FooPrefix() { this.getStringValue().substring("foo:") = offset };
13+
* override int getOffset() { result = offset }
14+
* };
15+
*
16+
* predicate mayFollowFoo(Expr e) { e = any(FooPrefix fp).getAnAppendedExpression() }
17+
* ```
18+
*
19+
* This will identify all the `suffix` expressions in contexts such as:
20+
*
21+
* ```
22+
* "foo:" + suffix1
23+
* "barfoo:" + suffix2
24+
* stringBuilder.append("foo:").append(suffix3);
25+
* String.format("%sfoo:%s", notSuffix, suffix4);
26+
* ```
27+
*/
28+
29+
import java
30+
private import semmle.code.java.dataflow.TaintTracking
31+
private import semmle.code.java.StringFormat
32+
33+
/**
34+
* A string constant that contains a prefix whose possibly-appended strings are
35+
* returned by `getAnAppendedExpression`.
36+
*
37+
* Extend this class to specify prefixes whose possibly-appended strings should be analysed.
38+
*/
39+
abstract class InterestingPrefix extends CompileTimeConstantExpr {
40+
/**
41+
* Gets the offset in this constant string where the interesting prefix begins.
42+
*/
43+
abstract int getOffset();
44+
45+
/**
46+
* Gets an expression that may follow this prefix in a derived string.
47+
*/
48+
Expr getAnAppendedExpression() { mayFollowInterestingPrefix(this, result) }
49+
}
50+
51+
private Expr getAnInterestingPrefix(InterestingPrefix root) {
52+
result = root
53+
or
54+
result.(AddExpr).getAnOperand() = getAnInterestingPrefix(root)
55+
}
56+
57+
private class StringBuilderAppend extends MethodAccess {
58+
StringBuilderAppend() {
59+
this.getMethod().getDeclaringType() instanceof StringBuildingType and
60+
this.getMethod().hasName("append")
61+
}
62+
}
63+
64+
private class StringBuilderConstructorOrAppend extends Call {
65+
StringBuilderConstructorOrAppend() {
66+
this instanceof StringBuilderAppend or
67+
this.(ClassInstanceExpr).getConstructedType() instanceof StringBuildingType
68+
}
69+
}
70+
71+
private Expr getQualifier(Expr e) { result = e.(MethodAccess).getQualifier() }
72+
73+
/**
74+
* An extension of `StringBuilderVar` that also accounts for strings appended in StringBuilder/Buffer's constructor
75+
* and in `append` calls chained onto the constructor call.
76+
*
77+
* The original `StringBuilderVar` doesn't care about these because it is designed to model taint, and
78+
* in taint rules terms these are not needed, as the connection between construction, appends and the
79+
* eventual `toString` is more obvious.
80+
*/
81+
private class StringBuilderVarExt extends StringBuilderVar {
82+
/**
83+
* Returns a first assignment after this StringBuilderVar is first assigned.
84+
*
85+
* For example, for `StringBuilder sbv = new StringBuilder("1").append("2"); sbv.append("3").append("4");`
86+
* this returns the append of `"3"`.
87+
*/
88+
private StringBuilderAppend getAFirstAppendAfterAssignment() {
89+
result = this.getAnAppend() and not result = this.getNextAppend(_)
90+
}
91+
92+
/**
93+
* Gets the next `append` after `prev`, where `prev` is, perhaps after some more `append` or other
94+
* chained calls, assigned to this `StringBuilderVar`.
95+
*/
96+
private StringBuilderAppend getNextAssignmentChainedAppend(StringBuilderConstructorOrAppend prev) {
97+
getQualifier*(result) = this.getAnAssignedValue() and
98+
result.getQualifier() = prev
99+
}
100+
101+
/**
102+
* Get a constructor call or `append` call that contributes a string to this string builder.
103+
*/
104+
StringBuilderConstructorOrAppend getAConstructorOrAppend() {
105+
exists(this.getNextAssignmentChainedAppend(result)) or
106+
result = this.getAnAssignedValue() or
107+
result = this.getAnAppend()
108+
}
109+
110+
/**
111+
* Like `StringBuilderVar.getNextAppend`, except including appends and constructors directly
112+
* assigned to this `StringBuilderVar`.
113+
*/
114+
private StringBuilderAppend getNextAppendIncludingAssignmentChains(
115+
StringBuilderConstructorOrAppend prev
116+
) {
117+
result = this.getNextAssignmentChainedAppend(prev)
118+
or
119+
prev = this.getAnAssignedValue() and
120+
result = this.getAFirstAppendAfterAssignment()
121+
or
122+
result = this.getNextAppend(prev)
123+
}
124+
125+
/**
126+
* Implements `StringBuilderVarExt.getNextAppendIncludingAssignmentChains+(prev)`.
127+
*/
128+
pragma[nomagic]
129+
StringBuilderAppend getSubsequentAppendIncludingAssignmentChains(
130+
StringBuilderConstructorOrAppend prev
131+
) {
132+
result = this.getNextAppendIncludingAssignmentChains(prev) or
133+
result =
134+
this.getSubsequentAppendIncludingAssignmentChains(this.getNextAppendIncludingAssignmentChains(prev))
135+
}
136+
}
137+
138+
/**
139+
* Holds if `follows` may be concatenated after `prefix`.
140+
*/
141+
private predicate mayFollowInterestingPrefix(InterestingPrefix prefix, Expr follows) {
142+
// Expressions that come after an interesting prefix in a tree of string additions:
143+
follows =
144+
any(AddExpr add | add.getLeftOperand() = getAnInterestingPrefix(prefix)).getRightOperand()
145+
or
146+
// Sanitize expressions that come after an interesting prefix in a sequence of StringBuilder operations:
147+
exists(
148+
StringBuilderConstructorOrAppend appendSanitizingConstant, StringBuilderAppend subsequentAppend,
149+
StringBuilderVarExt v
150+
|
151+
appendSanitizingConstant = v.getAConstructorOrAppend() and
152+
appendSanitizingConstant.getArgument(0) = getAnInterestingPrefix(prefix) and
153+
v.getSubsequentAppendIncludingAssignmentChains(appendSanitizingConstant) = subsequentAppend and
154+
follows = subsequentAppend.getArgument(0)
155+
)
156+
or
157+
// Sanitize expressions that come after an interesting prefix in the args to a format call:
158+
exists(
159+
FormattingCall formatCall, FormatString formatString, int prefixOffset, int laterOffset,
160+
int sanitizedArg
161+
|
162+
formatString = unique(FormatString fs | fs = formatCall.getAFormatString()) and
163+
(
164+
// An interesting prefix argument comes before this:
165+
exists(int argIdx |
166+
formatCall.getArgumentToBeFormatted(argIdx) = prefix and
167+
prefixOffset = formatString.getAnArgUsageOffset(argIdx)
168+
)
169+
or
170+
// The format string itself contains an interesting prefix that precedes subsequent arguments:
171+
formatString = prefix.getStringValue() and
172+
prefixOffset = prefix.getOffset()
173+
) and
174+
laterOffset > prefixOffset and
175+
laterOffset = formatString.getAnArgUsageOffset(sanitizedArg) and
176+
follows = formatCall.getArgumentToBeFormatted(sanitizedArg)
177+
)
178+
}

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

Lines changed: 5 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import semmle.code.java.frameworks.spring.Spring
77
import semmle.code.java.frameworks.JaxWS
88
import semmle.code.java.frameworks.javase.Http
99
import semmle.code.java.dataflow.DataFlow
10-
import semmle.code.java.dataflow.TaintTracking
11-
private import semmle.code.java.StringFormat
10+
private import semmle.code.java.dataflow.StringPrefixes
1211
private import semmle.code.java.dataflow.ExternalFlow
1312

1413
/**
@@ -52,10 +51,10 @@ private class PrimitiveSanitizer extends RequestForgerySanitizer {
5251
}
5352
}
5453

55-
private class HostnameSanitizingConstantPrefix extends CompileTimeConstantExpr {
54+
private class HostnameSanitizingPrefix extends InterestingPrefix {
5655
int offset;
5756

58-
HostnameSanitizingConstantPrefix() {
57+
HostnameSanitizingPrefix() {
5958
// Matches strings that look like when prepended to untrusted input, they will restrict
6059
// the host or entity addressed: for example, anything containing `?` or `#`, or a slash that
6160
// doesn't appear to be a protocol specifier (e.g. `http://` is not sanitizing), or specifically
@@ -66,149 +65,13 @@ private class HostnameSanitizingConstantPrefix extends CompileTimeConstantExpr {
6665
)
6766
}
6867

69-
/**
70-
* Gets the offset in this constant string where a sanitizing substring begins.
71-
*/
72-
int getOffset() { result = offset }
73-
}
74-
75-
private Expr getAHostnameSanitizingPrefix() {
76-
result instanceof HostnameSanitizingConstantPrefix
77-
or
78-
result.(AddExpr).getAnOperand() = getAHostnameSanitizingPrefix()
79-
}
80-
81-
private class StringBuilderAppend extends MethodAccess {
82-
StringBuilderAppend() {
83-
this.getMethod().getDeclaringType() instanceof StringBuildingType and
84-
this.getMethod().hasName("append")
85-
}
86-
}
87-
88-
private class StringBuilderConstructorOrAppend extends Call {
89-
StringBuilderConstructorOrAppend() {
90-
this instanceof StringBuilderAppend or
91-
this.(ClassInstanceExpr).getConstructedType() instanceof StringBuildingType
92-
}
93-
}
94-
95-
private Expr getQualifier(Expr e) { result = e.(MethodAccess).getQualifier() }
96-
97-
/**
98-
* An extension of `StringBuilderVar` that also accounts for strings appended in StringBuilder/Buffer's constructor
99-
* and in `append` calls chained onto the constructor call.
100-
*
101-
* The original `StringBuilderVar` doesn't care about these because it is designed to model taint, and
102-
* in taint rules terms these are not needed, as the connection between construction, appends and the
103-
* eventual `toString` is more obvious.
104-
*/
105-
private class StringBuilderVarExt extends StringBuilderVar {
106-
/**
107-
* Returns a first assignment after this StringBuilderVar is first assigned.
108-
*
109-
* For example, for `StringBuilder sbv = new StringBuilder("1").append("2"); sbv.append("3").append("4");`
110-
* this returns the append of `"3"`.
111-
*/
112-
private StringBuilderAppend getAFirstAppendAfterAssignment() {
113-
result = this.getAnAppend() and not result = this.getNextAppend(_)
114-
}
115-
116-
/**
117-
* Gets the next `append` after `prev`, where `prev` is, perhaps after some more `append` or other
118-
* chained calls, assigned to this `StringBuilderVar`.
119-
*/
120-
private StringBuilderAppend getNextAssignmentChainedAppend(StringBuilderConstructorOrAppend prev) {
121-
getQualifier*(result) = this.getAnAssignedValue() and
122-
result.getQualifier() = prev
123-
}
124-
125-
/**
126-
* Get a constructor call or `append` call that contributes a string to this string builder.
127-
*/
128-
StringBuilderConstructorOrAppend getAConstructorOrAppend() {
129-
exists(this.getNextAssignmentChainedAppend(result)) or
130-
result = this.getAnAssignedValue() or
131-
result = this.getAnAppend()
132-
}
133-
134-
/**
135-
* Like `StringBuilderVar.getNextAppend`, except including appends and constructors directly
136-
* assigned to this `StringBuilderVar`.
137-
*/
138-
private StringBuilderAppend getNextAppendIncludingAssignmentChains(
139-
StringBuilderConstructorOrAppend prev
140-
) {
141-
result = this.getNextAssignmentChainedAppend(prev)
142-
or
143-
prev = this.getAnAssignedValue() and
144-
result = this.getAFirstAppendAfterAssignment()
145-
or
146-
result = this.getNextAppend(prev)
147-
}
148-
149-
/**
150-
* Implements `StringBuilderVarExt.getNextAppendIncludingAssignmentChains+(prev)`.
151-
*/
152-
pragma[nomagic]
153-
StringBuilderAppend getSubsequentAppendIncludingAssignmentChains(
154-
StringBuilderConstructorOrAppend prev
155-
) {
156-
result = this.getNextAppendIncludingAssignmentChains(prev) or
157-
result =
158-
this.getSubsequentAppendIncludingAssignmentChains(this.getNextAppendIncludingAssignmentChains(prev))
159-
}
160-
}
161-
162-
/**
163-
* An expression that is sanitized because it is concatenated onto a string that looks like
164-
* a hostname or a URL separator, preventing the appended string from arbitrarily controlling
165-
* the addressed server.
166-
*/
167-
private class HostnameSanitizedExpr extends Expr {
168-
HostnameSanitizedExpr() {
169-
// Sanitize expressions that come after a sanitizing prefix in a tree of string additions:
170-
this =
171-
any(AddExpr add | add.getLeftOperand() = getAHostnameSanitizingPrefix()).getRightOperand()
172-
or
173-
// Sanitize expressions that come after a sanitizing prefix in a sequence of StringBuilder operations:
174-
exists(
175-
StringBuilderConstructorOrAppend appendSanitizingConstant,
176-
StringBuilderAppend subsequentAppend, StringBuilderVarExt v
177-
|
178-
appendSanitizingConstant = v.getAConstructorOrAppend() and
179-
appendSanitizingConstant.getArgument(0) = getAHostnameSanitizingPrefix() and
180-
v.getSubsequentAppendIncludingAssignmentChains(appendSanitizingConstant) = subsequentAppend and
181-
this = subsequentAppend.getArgument(0)
182-
)
183-
or
184-
// Sanitize expressions that come after a sanitizing prefix in the args to a format call:
185-
exists(
186-
FormattingCall formatCall, FormatString formatString, HostnameSanitizingConstantPrefix prefix,
187-
int sanitizedFromOffset, int laterOffset, int sanitizedArg
188-
|
189-
formatString = unique(FormatString fs | fs = formatCall.getAFormatString()) and
190-
(
191-
// A sanitizing argument comes before this:
192-
exists(int argIdx |
193-
formatCall.getArgumentToBeFormatted(argIdx) = prefix and
194-
sanitizedFromOffset = formatString.getAnArgUsageOffset(argIdx)
195-
)
196-
or
197-
// The format string itself sanitizes subsequent arguments:
198-
formatString = prefix.getStringValue() and
199-
sanitizedFromOffset = prefix.getOffset()
200-
) and
201-
laterOffset > sanitizedFromOffset and
202-
laterOffset = formatString.getAnArgUsageOffset(sanitizedArg) and
203-
this = formatCall.getArgumentToBeFormatted(sanitizedArg)
204-
)
205-
}
68+
override int getOffset() { result = offset }
20669
}
20770

20871
/**
20972
* A value that is the result of prepending a string that prevents any value from controlling the
21073
* host of a URL.
21174
*/
21275
private class HostnameSantizer extends RequestForgerySanitizer {
213-
HostnameSantizer() { this.asExpr() instanceof HostnameSanitizedExpr }
76+
HostnameSantizer() { this.asExpr() = any(HostnameSanitizingPrefix hsp).getAnAppendedExpression() }
21477
}

0 commit comments

Comments
 (0)