@@ -3,11 +3,141 @@ import DataFlow
33import semmle.code.java.dataflow.FlowSources
44import semmle.code.java.frameworks.Servlets
55
6+ /** A sanitizer for unsafe url forward vulnerabilities. */
7+ abstract class UnsafeUrlForwardSanitizer extends DataFlow:: Node { }
8+
9+ private class PrimitiveSanitizer extends UnsafeUrlForwardSanitizer {
10+ PrimitiveSanitizer ( ) {
11+ this .getType ( ) instanceof PrimitiveType or
12+ this .getType ( ) instanceof BoxedType or
13+ this .getType ( ) instanceof NumberType
14+ }
15+ }
16+
17+ private class UnsafeUrlForwardSantizer extends UnsafeUrlForwardSanitizer {
18+ UnsafeUrlForwardSantizer ( ) { this .asExpr ( ) instanceof UnsafeUrlForwardSanitizedExpr }
19+ }
20+
21+ private class UnsafeUrlForwardSanitizingConstantPrefix extends CompileTimeConstantExpr {
22+ UnsafeUrlForwardSanitizingConstantPrefix ( ) {
23+ not this .getStringValue ( ) .matches ( "/WEB-INF/%" ) and
24+ not this .getStringValue ( ) = "forward:"
25+ }
26+ }
27+
28+ private Expr getAUnsafeUrlForwardSanitizingPrefix ( ) {
29+ result instanceof UnsafeUrlForwardSanitizingConstantPrefix
30+ or
31+ result .( AddExpr ) .getAnOperand ( ) = getAUnsafeUrlForwardSanitizingPrefix ( )
32+ }
33+
634/** A call to `StringBuilder.append` method. */
7- class StringBuilderAppendCall extends MethodAccess {
8- StringBuilderAppendCall ( ) {
9- this .getMethod ( ) .hasName ( "append" ) and
10- this .getMethod ( ) .getDeclaringType ( ) instanceof StringBuildingType
35+ class StringBuilderAppend extends MethodAccess {
36+ StringBuilderAppend ( ) {
37+ this .getMethod ( ) .getDeclaringType ( ) instanceof StringBuildingType and
38+ this .getMethod ( ) .hasName ( "append" )
39+ }
40+ }
41+
42+ private Expr getQualifier ( Expr e ) { result = e .( MethodAccess ) .getQualifier ( ) }
43+
44+ /**
45+ * An extension of `StringBuilderVar` that also accounts for strings appended in StringBuilder/Buffer's constructor
46+ * and in `append` calls chained onto the constructor call.
47+ *
48+ * The original `StringBuilderVar` doesn't care about these because it is designed to model taint, and
49+ * in taint rules terms these are not needed, as the connection between construction, appends and the
50+ * eventual `toString` is more obvious.
51+ */
52+ private class StringBuilderVarExt extends StringBuilderVar {
53+ /**
54+ * Returns a first assignment after this StringBuilderVar is first assigned.
55+ *
56+ * For example, for `StringBuilder sbv = new StringBuilder("1").append("2"); sbv.append("3").append("4");`
57+ * this returns the append of `"3"`.
58+ */
59+ private StringBuilderAppend getAFirstAppendAfterAssignment ( ) {
60+ result = this .getAnAppend ( ) and not result = this .getNextAppend ( _)
61+ }
62+
63+ /**
64+ * Gets the next `append` after `prev`, where `prev` is, perhaps after some more `append` or other
65+ * chained calls, assigned to this `StringBuilderVar`.
66+ */
67+ private StringBuilderAppend getNextAssignmentChainedAppend ( StringBuilderConstructorOrAppend prev ) {
68+ getQualifier * ( result ) = this .getAnAssignedValue ( ) and
69+ result .getQualifier ( ) = prev
70+ }
71+
72+ /**
73+ * Get a constructor call or `append` call that contributes a string to this string builder.
74+ */
75+ StringBuilderConstructorOrAppend getAConstructorOrAppend ( ) {
76+ exists ( this .getNextAssignmentChainedAppend ( result ) ) or
77+ result = this .getAnAssignedValue ( ) or
78+ result = this .getAnAppend ( )
79+ }
80+
81+ /**
82+ * Like `StringBuilderVar.getNextAppend`, except including appends and constructors directly
83+ * assigned to this `StringBuilderVar`.
84+ */
85+ private StringBuilderAppend getNextAppendIncludingAssignmentChains (
86+ StringBuilderConstructorOrAppend prev
87+ ) {
88+ result = getNextAssignmentChainedAppend ( prev )
89+ or
90+ prev = this .getAnAssignedValue ( ) and
91+ result = this .getAFirstAppendAfterAssignment ( )
92+ or
93+ result = this .getNextAppend ( prev )
94+ }
95+
96+ /**
97+ * Implements `StringBuilderVarExt.getNextAppendIncludingAssignmentChains+(prev)`.
98+ */
99+ pragma [ nomagic]
100+ StringBuilderAppend getSubsequentAppendIncludingAssignmentChains (
101+ StringBuilderConstructorOrAppend prev
102+ ) {
103+ result = this .getNextAppendIncludingAssignmentChains ( prev ) or
104+ result =
105+ this .getSubsequentAppendIncludingAssignmentChains ( this .getNextAppendIncludingAssignmentChains ( prev ) )
106+ }
107+ }
108+
109+ private class StringBuilderConstructorOrAppend extends Call {
110+ StringBuilderConstructorOrAppend ( ) {
111+ this instanceof StringBuilderAppend or
112+ this .( ClassInstanceExpr ) .getConstructedType ( ) instanceof StringBuildingType
113+ }
114+ }
115+
116+ private class UnsafeUrlForwardSanitizedExpr extends Expr {
117+ UnsafeUrlForwardSanitizedExpr ( ) {
118+ // Sanitize expressions that come after a sanitizing prefix in a tree of string additions:
119+ this =
120+ any ( AddExpr add | add .getLeftOperand ( ) = getAUnsafeUrlForwardSanitizingPrefix ( ) )
121+ .getRightOperand ( )
122+ or
123+ // Sanitize expressions that come after a sanitizing prefix in a sequence of StringBuilder operations:
124+ exists (
125+ StringBuilderConstructorOrAppend appendSanitizingConstant ,
126+ StringBuilderAppend subsequentAppend , StringBuilderVarExt v
127+ |
128+ appendSanitizingConstant = v .getAConstructorOrAppend ( ) and
129+ appendSanitizingConstant .getArgument ( 0 ) = getAUnsafeUrlForwardSanitizingPrefix ( ) and
130+ v .getSubsequentAppendIncludingAssignmentChains ( appendSanitizingConstant ) = subsequentAppend and
131+ this = subsequentAppend .getArgument ( 0 )
132+ )
133+ or
134+ exists ( MethodAccess ma , int i |
135+ ma .getMethod ( ) .hasName ( "format" ) and
136+ ma .getMethod ( ) .getDeclaringType ( ) instanceof TypeString and
137+ ma .getArgument ( 0 ) instanceof UnsafeUrlForwardSanitizingConstantPrefix and
138+ ma .getArgument ( i ) = this and
139+ i != 0
140+ )
11141 }
12142}
13143
@@ -16,7 +146,7 @@ class StringBuilderAppendCall extends MethodAccess {
16146 *
17147 * E.g: `"forward:" + url`
18148 */
19- class ForwardBuilderExpr extends AddExpr {
149+ private class ForwardBuilderExpr extends AddExpr {
20150 ForwardBuilderExpr ( ) {
21151 this .getLeftOperand ( ) .( CompileTimeConstantExpr ) .getStringValue ( ) = "forward:"
22152 }
@@ -27,7 +157,7 @@ class ForwardBuilderExpr extends AddExpr {
27157 *
28158 * E.g: `StringBuilder.append("forward:")`
29159 */
30- class ForwardAppendCall extends StringBuilderAppendCall {
160+ private class ForwardAppendCall extends StringBuilderAppend {
31161 ForwardAppendCall ( ) {
32162 this .getArgument ( 0 ) .( CompileTimeConstantExpr ) .getStringValue ( ) = "forward:"
33163 }
0 commit comments