Skip to content

Commit 248628b

Browse files
committed
Enhance basic auth string search with a recursive method
1 parent 3a23451 commit 248628b

File tree

1 file changed

+22
-28
lines changed

1 file changed

+22
-28
lines changed

java/ql/src/experimental/Security/CWE/CWE-522/UnsecureBasicAuth.ql

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,26 @@ predicate builtFromHttpStringConcat(Expr expr) {
9898
)
9999
}
100100

101+
/**
102+
* String pattern of basic authentication.
103+
*/
104+
class BasicAuthString extends StringLiteral {
105+
BasicAuthString() { exists(string s | this.getRepresentedString() = s | s.matches("Basic %")) }
106+
}
107+
108+
/**
109+
* String concatenated with `BasicAuthString`.
110+
*/
111+
predicate builtFromBasicAuthStringConcat(Expr expr) {
112+
expr instanceof BasicAuthString
113+
or
114+
builtFromBasicAuthStringConcat(expr.(AddExpr).getLeftOperand())
115+
or
116+
exists(Expr other | builtFromBasicAuthStringConcat(other) |
117+
exists(Variable var | var.getAnAssignedValue() = other and var.getAnAccess() = expr)
118+
)
119+
}
120+
101121
/**
102122
* The methods `addHeader()` and `setHeader` declared in ApacheHttpRequest invoked for basic authentication.
103123
*/
@@ -106,20 +126,7 @@ class AddBasicAuthHeaderMethodAccess extends MethodAccess {
106126
this.getMethod().getDeclaringType() instanceof ApacheHttpRequest and
107127
(this.getMethod().hasName("addHeader") or this.getMethod().hasName("setHeader")) and
108128
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and
109-
exists(Expr arg1 |
110-
arg1 = this.getArgument(1) and //Check three patterns
111-
(
112-
arg1 //String authStringEnc = "Basic ...."; post.addHeader("Authorization", authStringEnc)
113-
.(VarAccess)
114-
.getVariable()
115-
.getAnAssignedValue()
116-
.(StringLiteral)
117-
.getRepresentedString()
118-
.matches("Basic %") or
119-
arg1.(CompileTimeConstantExpr).getStringValue().matches("Basic %") or //post.addHeader("Authorization", "Basic ....")
120-
arg1.(AddExpr).getLeftOperand().(StringLiteral).getRepresentedString().matches("Basic %") //post.addHeader("Authorization", "Basic "+authStringEnc)
121-
)
122-
) and
129+
builtFromBasicAuthStringConcat(this.getArgument(1)) and
123130
exists(VarAccess request, VariableAssign va, ConstructorCall cc, Expr arg0 |
124131
this.getQualifier() = request and // Constructor call like HttpPost post = new HttpPost("http://www.example.com/rest/endpoint.do"); and BasicHttpRequest post = new BasicHttpRequest("POST", uriStr);
125132
va.getDestVar() = request.getVariable() and
@@ -204,20 +211,7 @@ class SetBasicAuthPropertyMethodAccess extends MethodAccess {
204211
this.getMethod().getDeclaringType() instanceof TypeUrlConnection and
205212
this.getMethod().hasName("setRequestProperty") and
206213
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and
207-
exists(Expr arg1 |
208-
arg1 = this.getArgument(1) and //Check three patterns
209-
(
210-
arg1 //String authStringEnc = "Basic ...."; conn.setRequestProperty("Authorization", authStringEnc)
211-
.(VarAccess)
212-
.getVariable()
213-
.getAnAssignedValue()
214-
.(StringLiteral)
215-
.getRepresentedString()
216-
.matches("Basic %") or
217-
arg1.(CompileTimeConstantExpr).getStringValue().matches("Basic %") or //conn.setRequestProperty("Authorization", "Basic ....")
218-
arg1.(AddExpr).getLeftOperand().(StringLiteral).getRepresentedString().matches("Basic %") //conn.setRequestProperty("Authorization", "Basic "+authStringEnc)
219-
)
220-
) and
214+
builtFromBasicAuthStringConcat(this.getArgument(1)) and
221215
exists(VarAccess conn, DataFlow::PathNode source, DataFlow::PathNode sink, HttpString s |
222216
this.getQualifier() = conn and //HttpURLConnection conn = (HttpURLConnection) url.openConnection();
223217
source.getNode().asExpr() = s and

0 commit comments

Comments
 (0)