Skip to content

Commit 5b25694

Browse files
committed
Simplify and improve AddExpr logic
The improvement is in considering (userSupplied + "/") itself a sanitising prefix.
1 parent 6b76f42 commit 5b25694

File tree

1 file changed

+12
-21
lines changed

1 file changed

+12
-21
lines changed

java/ql/src/semmle/code/java/security/RequestForgery.qll

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -222,10 +222,10 @@ private class PrimitiveSanitizer extends RequestForgerySanitizer {
222222
}
223223
}
224224

225-
private class HostnameSanitizingPrefix extends CompileTimeConstantExpr {
225+
private class HostnameSanitizingConstantPrefix extends CompileTimeConstantExpr {
226226
int offset;
227227

228-
HostnameSanitizingPrefix() {
228+
HostnameSanitizingConstantPrefix() {
229229
// Matches strings that look like when prepended to untrusted input, they will restrict
230230
// the host or entity addressed: for example, anything containing `?` or `#`, or a slash that
231231
// doesn't appear to be a protocol specifier (e.g. `http://` is not sanitizing), or specifically
@@ -242,21 +242,10 @@ private class HostnameSanitizingPrefix extends CompileTimeConstantExpr {
242242
int getOffset() { result = offset }
243243
}
244244

245-
private AddExpr getParentAdd(AddExpr e) { result = e.getParent() }
246-
247-
private AddExpr getAnAddContainingHostnameSanitizingPrefix() {
248-
result = getParentAdd*(any(HostnameSanitizingPrefix p).getParent())
249-
}
250-
251-
private Expr getASanitizedAddOperand() {
252-
exists(AddExpr e |
253-
e = getAnAddContainingHostnameSanitizingPrefix() and
254-
(
255-
e.getLeftOperand() = getAnAddContainingHostnameSanitizingPrefix() or
256-
e.getLeftOperand() instanceof HostnameSanitizingPrefix
257-
) and
258-
result = e.getRightOperand()
259-
)
245+
private Expr getAHostnameSanitizingPrefix() {
246+
result instanceof HostnameSanitizingConstantPrefix
247+
or
248+
result.(AddExpr).getAnOperand() = getAHostnameSanitizingPrefix()
260249
}
261250

262251
private MethodAccess getNextAppend(MethodAccess append) {
@@ -283,15 +272,16 @@ private MethodAccess getAChainedAppend(Expr e) {
283272
private class HostnameSanitizedExpr extends Expr {
284273
HostnameSanitizedExpr() {
285274
// Sanitize expressions that come after a sanitizing prefix in a tree of string additions:
286-
this = getASanitizedAddOperand()
275+
this =
276+
any(AddExpr add | add.getLeftOperand() = getAHostnameSanitizingPrefix()).getRightOperand()
287277
or
288278
// Sanitize all appends to a StringBuilder that is initialized with a sanitizing prefix:
289279
// (note imprecision: if the same StringBuilder/StringBuffer has more than one constructor call,
290280
// this sanitizes all of its append calls, not just those that may follow the constructor).
291281
exists(StringBuilderVar sbv, ConstructorCall constructor, Expr initializer |
292282
initializer = sbv.getAnAssignedValue() and
293283
constructor = getQualifier*(initializer) and
294-
constructor.getArgument(0) instanceof HostnameSanitizingPrefix and
284+
constructor.getArgument(0) = getAHostnameSanitizingPrefix() and
295285
(
296286
this = sbv.getAnAppend().getArgument(0)
297287
or
@@ -301,14 +291,15 @@ private class HostnameSanitizedExpr extends Expr {
301291
or
302292
// Sanitize expressions that come after a sanitizing prefix in a sequence of StringBuilder operations:
303293
exists(MethodAccess appendSanitizingConstant, MethodAccess subsequentAppend |
304-
appendSanitizingConstant.getArgument(0) instanceof HostnameSanitizingPrefix and
294+
appendSanitizingConstant = any(StringBuilderVar v).getAnAppend() and
295+
appendSanitizingConstant.getArgument(0) = getAHostnameSanitizingPrefix() and
305296
getNextAppend*(appendSanitizingConstant) = subsequentAppend and
306297
this = subsequentAppend.getArgument(0)
307298
)
308299
or
309300
// Sanitize expressions that come after a sanitizing prefix in the args to a format call:
310301
exists(
311-
FormattingCall formatCall, FormatString formatString, HostnameSanitizingPrefix prefix,
302+
FormattingCall formatCall, FormatString formatString, HostnameSanitizingConstantPrefix prefix,
312303
int sanitizedFromOffset, int laterOffset, int sanitizedArg
313304
|
314305
formatString = unique(FormatString fs | fs = formatCall.getAFormatString()) and

0 commit comments

Comments
 (0)