diff --git a/java/ql/lib/semmle/code/java/dataflow/StringPrefixes.qll b/java/ql/lib/semmle/code/java/dataflow/StringPrefixes.qll new file mode 100644 index 00000000000..ac374ccaf72 --- /dev/null +++ b/java/ql/lib/semmle/code/java/dataflow/StringPrefixes.qll @@ -0,0 +1,151 @@ +/** + * Provides classes and predicates for identifying expressions that may be appended to an interesting prefix. + * + * To use this library, extend the abstract class `InterestingPrefix` to have the library identify expressions that + * may be appended to it, then check `InterestingPrefix.getAnAppendedExpression(Expr)` to get your results. + * + * For example, `private class FooPrefix extends InterestingPrefix { FooPrefix() { this = "foo:" } };` + * `predicate mayFollowFoo(Expr e) { e = any(FooPrefix fp).getAnAppendedExpression() }` + */ + +import java +import semmle.code.java.dataflow.TaintTracking +private import semmle.code.java.StringFormat + +abstract class InterestingPrefix extends CompileTimeConstantExpr { + /** + * Gets the offset in this constant string where the interesting substring begins. + */ + abstract int getOffset(); + + Expr getAnAppendedExpression() { mayFollowInterestingPrefix(this, result) } +} + +private Expr getAnInterestingPrefix(InterestingPrefix root) { + result = root + or + result.(AddExpr).getAnOperand() = getAnInterestingPrefix(root) +} + +private class StringBuilderAppend extends MethodAccess { + StringBuilderAppend() { + this.getMethod().getDeclaringType() instanceof StringBuildingType and + this.getMethod().hasName("append") + } +} + +private class StringBuilderConstructorOrAppend extends Call { + StringBuilderConstructorOrAppend() { + this instanceof StringBuilderAppend or + this.(ClassInstanceExpr).getConstructedType() instanceof StringBuildingType + } +} + +private Expr getQualifier(Expr e) { result = e.(MethodAccess).getQualifier() } + +/** + * An extension of `StringBuilderVar` that also accounts for strings appended in StringBuilder/Buffer's constructor + * and in `append` calls chained onto the constructor call. + * + * The original `StringBuilderVar` doesn't care about these because it is designed to model taint, and + * in taint rules terms these are not needed, as the connection between construction, appends and the + * eventual `toString` is more obvious. + */ +private class StringBuilderVarExt extends StringBuilderVar { + /** + * Returns a first assignment after this StringBuilderVar is first assigned. + * + * For example, for `StringBuilder sbv = new StringBuilder("1").append("2"); sbv.append("3").append("4");` + * this returns the append of `"3"`. + */ + private StringBuilderAppend getAFirstAppendAfterAssignment() { + result = this.getAnAppend() and not result = this.getNextAppend(_) + } + + /** + * Gets the next `append` after `prev`, where `prev` is, perhaps after some more `append` or other + * chained calls, assigned to this `StringBuilderVar`. + */ + private StringBuilderAppend getNextAssignmentChainedAppend(StringBuilderConstructorOrAppend prev) { + getQualifier*(result) = this.getAnAssignedValue() and + result.getQualifier() = prev + } + + /** + * Get a constructor call or `append` call that contributes a string to this string builder. + */ + StringBuilderConstructorOrAppend getAConstructorOrAppend() { + exists(this.getNextAssignmentChainedAppend(result)) or + result = this.getAnAssignedValue() or + result = this.getAnAppend() + } + + /** + * Like `StringBuilderVar.getNextAppend`, except including appends and constructors directly + * assigned to this `StringBuilderVar`. + */ + private StringBuilderAppend getNextAppendIncludingAssignmentChains( + StringBuilderConstructorOrAppend prev + ) { + result = getNextAssignmentChainedAppend(prev) + or + prev = this.getAnAssignedValue() and + result = this.getAFirstAppendAfterAssignment() + or + result = this.getNextAppend(prev) + } + + /** + * Implements `StringBuilderVarExt.getNextAppendIncludingAssignmentChains+(prev)`. + */ + pragma[nomagic] + StringBuilderAppend getSubsequentAppendIncludingAssignmentChains( + StringBuilderConstructorOrAppend prev + ) { + result = this.getNextAppendIncludingAssignmentChains(prev) or + result = + this.getSubsequentAppendIncludingAssignmentChains(this.getNextAppendIncludingAssignmentChains(prev)) + } +} + +/** + * Holds if `follows` may be concatenated after `prefix`. + */ +private predicate mayFollowInterestingPrefix(InterestingPrefix prefix, Expr follows) { + // Expressions that come after an interesting prefix in a tree of string additions: + follows = + any(AddExpr add | add.getLeftOperand() = getAnInterestingPrefix(prefix)).getRightOperand() + or + // Sanitize expressions that come after an interesting prefix in a sequence of StringBuilder operations: + exists( + StringBuilderConstructorOrAppend appendSanitizingConstant, StringBuilderAppend subsequentAppend, + StringBuilderVarExt v + | + appendSanitizingConstant = v.getAConstructorOrAppend() and + appendSanitizingConstant.getArgument(0) = getAnInterestingPrefix(prefix) and + v.getSubsequentAppendIncludingAssignmentChains(appendSanitizingConstant) = subsequentAppend and + follows = subsequentAppend.getArgument(0) + ) + or + // Sanitize expressions that come after an interesting prefix in the args to a format call: + exists( + FormattingCall formatCall, FormatString formatString, int prefixOffset, int laterOffset, + int sanitizedArg + | + formatString = unique(FormatString fs | fs = formatCall.getAFormatString()) and + ( + // An interesting prefix argument comes before this: + exists(int argIdx | + formatCall.getArgumentToBeFormatted(argIdx) = prefix and + prefixOffset = formatString.getAnArgUsageOffset(argIdx) + ) + or + // The format string itself contains an interesting prefix that precedes subsequent arguments: + formatString = prefix.getStringValue() and + prefixOffset = prefix.getOffset() + ) and + laterOffset > prefixOffset and + laterOffset = formatString.getAnArgUsageOffset(sanitizedArg) and + follows = formatCall.getArgumentToBeFormatted(sanitizedArg) + ) +} diff --git a/java/ql/lib/semmle/code/java/security/RequestForgery.qll b/java/ql/lib/semmle/code/java/security/RequestForgery.qll index 268bd364b15..be2f529d6ad 100644 --- a/java/ql/lib/semmle/code/java/security/RequestForgery.qll +++ b/java/ql/lib/semmle/code/java/security/RequestForgery.qll @@ -7,8 +7,7 @@ import semmle.code.java.frameworks.spring.Spring import semmle.code.java.frameworks.JaxWS import semmle.code.java.frameworks.javase.Http import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.TaintTracking -private import semmle.code.java.StringFormat +import semmle.code.java.dataflow.StringPrefixes private import semmle.code.java.dataflow.ExternalFlow /** @@ -52,10 +51,10 @@ private class PrimitiveSanitizer extends RequestForgerySanitizer { } } -private class HostnameSanitizingConstantPrefix extends CompileTimeConstantExpr { +private class HostnameSanitizingPrefix extends InterestingPrefix { int offset; - HostnameSanitizingConstantPrefix() { + HostnameSanitizingPrefix() { // Matches strings that look like when prepended to untrusted input, they will restrict // the host or entity addressed: for example, anything containing `?` or `#`, or a slash that // doesn't appear to be a protocol specifier (e.g. `http://` is not sanitizing), or specifically @@ -66,143 +65,7 @@ private class HostnameSanitizingConstantPrefix extends CompileTimeConstantExpr { ) } - /** - * Gets the offset in this constant string where a sanitizing substring begins. - */ - int getOffset() { result = offset } -} - -private Expr getAHostnameSanitizingPrefix() { - result instanceof HostnameSanitizingConstantPrefix - or - result.(AddExpr).getAnOperand() = getAHostnameSanitizingPrefix() -} - -private class StringBuilderAppend extends MethodAccess { - StringBuilderAppend() { - this.getMethod().getDeclaringType() instanceof StringBuildingType and - this.getMethod().hasName("append") - } -} - -private class StringBuilderConstructorOrAppend extends Call { - StringBuilderConstructorOrAppend() { - this instanceof StringBuilderAppend or - this.(ClassInstanceExpr).getConstructedType() instanceof StringBuildingType - } -} - -private Expr getQualifier(Expr e) { result = e.(MethodAccess).getQualifier() } - -/** - * An extension of `StringBuilderVar` that also accounts for strings appended in StringBuilder/Buffer's constructor - * and in `append` calls chained onto the constructor call. - * - * The original `StringBuilderVar` doesn't care about these because it is designed to model taint, and - * in taint rules terms these are not needed, as the connection between construction, appends and the - * eventual `toString` is more obvious. - */ -private class StringBuilderVarExt extends StringBuilderVar { - /** - * Returns a first assignment after this StringBuilderVar is first assigned. - * - * For example, for `StringBuilder sbv = new StringBuilder("1").append("2"); sbv.append("3").append("4");` - * this returns the append of `"3"`. - */ - private StringBuilderAppend getAFirstAppendAfterAssignment() { - result = this.getAnAppend() and not result = this.getNextAppend(_) - } - - /** - * Gets the next `append` after `prev`, where `prev` is, perhaps after some more `append` or other - * chained calls, assigned to this `StringBuilderVar`. - */ - private StringBuilderAppend getNextAssignmentChainedAppend(StringBuilderConstructorOrAppend prev) { - getQualifier*(result) = this.getAnAssignedValue() and - result.getQualifier() = prev - } - - /** - * Get a constructor call or `append` call that contributes a string to this string builder. - */ - StringBuilderConstructorOrAppend getAConstructorOrAppend() { - exists(this.getNextAssignmentChainedAppend(result)) or - result = this.getAnAssignedValue() or - result = this.getAnAppend() - } - - /** - * Like `StringBuilderVar.getNextAppend`, except including appends and constructors directly - * assigned to this `StringBuilderVar`. - */ - private StringBuilderAppend getNextAppendIncludingAssignmentChains( - StringBuilderConstructorOrAppend prev - ) { - result = this.getNextAssignmentChainedAppend(prev) - or - prev = this.getAnAssignedValue() and - result = this.getAFirstAppendAfterAssignment() - or - result = this.getNextAppend(prev) - } - - /** - * Implements `StringBuilderVarExt.getNextAppendIncludingAssignmentChains+(prev)`. - */ - pragma[nomagic] - StringBuilderAppend getSubsequentAppendIncludingAssignmentChains( - StringBuilderConstructorOrAppend prev - ) { - result = this.getNextAppendIncludingAssignmentChains(prev) or - result = - this.getSubsequentAppendIncludingAssignmentChains(this.getNextAppendIncludingAssignmentChains(prev)) - } -} - -/** - * An expression that is sanitized because it is concatenated onto a string that looks like - * a hostname or a URL separator, preventing the appended string from arbitrarily controlling - * the addressed server. - */ -private class HostnameSanitizedExpr extends Expr { - HostnameSanitizedExpr() { - // Sanitize expressions that come after a sanitizing prefix in a tree of string additions: - this = - any(AddExpr add | add.getLeftOperand() = getAHostnameSanitizingPrefix()).getRightOperand() - or - // Sanitize expressions that come after a sanitizing prefix in a sequence of StringBuilder operations: - exists( - StringBuilderConstructorOrAppend appendSanitizingConstant, - StringBuilderAppend subsequentAppend, StringBuilderVarExt v - | - appendSanitizingConstant = v.getAConstructorOrAppend() and - appendSanitizingConstant.getArgument(0) = getAHostnameSanitizingPrefix() and - v.getSubsequentAppendIncludingAssignmentChains(appendSanitizingConstant) = subsequentAppend and - this = subsequentAppend.getArgument(0) - ) - or - // Sanitize expressions that come after a sanitizing prefix in the args to a format call: - exists( - FormattingCall formatCall, FormatString formatString, HostnameSanitizingConstantPrefix prefix, - int sanitizedFromOffset, int laterOffset, int sanitizedArg - | - formatString = unique(FormatString fs | fs = formatCall.getAFormatString()) and - ( - // A sanitizing argument comes before this: - exists(int argIdx | - formatCall.getArgumentToBeFormatted(argIdx) = prefix and - sanitizedFromOffset = formatString.getAnArgUsageOffset(argIdx) - ) - or - // The format string itself sanitizes subsequent arguments: - formatString = prefix.getStringValue() and - sanitizedFromOffset = prefix.getOffset() - ) and - laterOffset > sanitizedFromOffset and - laterOffset = formatString.getAnArgUsageOffset(sanitizedArg) and - this = formatCall.getArgumentToBeFormatted(sanitizedArg) - ) - } + override int getOffset() { result = offset } } /** @@ -210,5 +73,5 @@ private class HostnameSanitizedExpr extends Expr { * host of a URL. */ private class HostnameSantizer extends RequestForgerySanitizer { - HostnameSantizer() { this.asExpr() instanceof HostnameSanitizedExpr } + HostnameSantizer() { this.asExpr() = any(HostnameSanitizingPrefix hsp).getAnAppendedExpression() } }