Skip to content

Commit b719202

Browse files
smowtonsmowton
committed
Factor out string prefix logic
1 parent b975e12 commit b719202

File tree

2 files changed

+156
-142
lines changed

2 files changed

+156
-142
lines changed

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

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

‎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+
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)