Skip to content

Commit 48975fa

Browse files
committed
Replace sanitizers
1 parent 31eaa80 commit 48975fa

File tree

1 file changed

+41
-64
lines changed

1 file changed

+41
-64
lines changed

java/ql/src/experimental/Security/CWE/CWE-1004/SensitiveCookieNotHttpOnly.ql

Lines changed: 41 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,6 @@ class CookieClass extends RefType {
5050
}
5151
}
5252

53-
/** The method call `toString` to get a stringified cookie representation. */
54-
class CookieInstanceExpr extends TaintPreservingCallable {
55-
CookieInstanceExpr() {
56-
this.getDeclaringType() instanceof CookieClass and
57-
this.hasName("toString")
58-
}
59-
60-
override predicate returnsTaintFrom(int arg) { arg = -1 }
61-
}
62-
63-
/** The cookie constructor. */
64-
class CookieTaintPreservingConstructor extends Constructor, TaintPreservingCallable {
65-
CookieTaintPreservingConstructor() { this.getDeclaringType() instanceof CookieClass }
66-
67-
override predicate returnsTaintFrom(int arg) { arg = 0 }
68-
}
69-
7053
/** Sensitive cookie name used in a `Cookie` constructor or a `Set-Cookie` call. */
7154
class SensitiveCookieNameExpr extends Expr {
7255
SensitiveCookieNameExpr() { isSensitiveCookieNameExpr(this) }
@@ -78,55 +61,58 @@ class CookieResponseSink extends DataFlow::ExprNode {
7861
exists(MethodAccess ma |
7962
(
8063
ma.getMethod() instanceof ResponseAddCookieMethod and
81-
this.getExpr() = ma.getArgument(0)
64+
this.getExpr() = ma.getArgument(0) and
65+
not exists(
66+
MethodAccess ma2 // cookie.setHttpOnly(true)
67+
|
68+
ma2.getMethod().getName() = "setHttpOnly" and
69+
ma2.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
70+
DataFlow::localExprFlow(ma2.getQualifier(), this.getExpr())
71+
)
8272
or
8373
ma instanceof SetCookieMethodAccess and
84-
this.getExpr() = ma.getArgument(1)
74+
this.getExpr() = ma.getArgument(1) and
75+
not hasHttpOnlyExpr(this.getExpr()) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure")
8576
)
8677
)
8778
}
8879
}
8980

90-
/**
91-
* Holds if `node` is an access to a variable which has `setHttpOnly(true)` called on it and is also
92-
* the first argument to a call to the method `addCookie` of `javax.servlet.http.HttpServletResponse`.
93-
*/
94-
predicate setHttpOnlyMethodAccess(DataFlow::Node node) {
95-
exists(
96-
MethodAccess addCookie, Variable cookie, MethodAccess m // jwtCookie.setHttpOnly(true)
97-
|
98-
addCookie.getMethod() instanceof ResponseAddCookieMethod and
99-
addCookie.getArgument(0) = cookie.getAnAccess() and
100-
m.getMethod().getName() = "setHttpOnly" and
101-
m.getArgument(0).(BooleanLiteral).getBooleanValue() = true and
102-
m.getQualifier() = cookie.getAnAccess() and
103-
node.asExpr() = cookie.getAnAccess()
104-
)
81+
/** A JAX-RS `NewCookie` constructor that sets `HttpOnly` to true. */
82+
class HttpOnlyNewCookie extends ClassInstanceExpr {
83+
HttpOnlyNewCookie() {
84+
this.getConstructedType()
85+
.hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "NewCookie") and
86+
(
87+
this.getNumArgument() = 6 and this.getArgument(5).(BooleanLiteral).getBooleanValue() = true // NewCookie(Cookie cookie, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
88+
or
89+
this.getNumArgument() = 8 and
90+
this.getArgument(6).getType() instanceof BooleanType and
91+
this.getArgument(7).(BooleanLiteral).getBooleanValue() = true // NewCookie(String name, String value, String path, String domain, String comment, int maxAge, boolean secure, boolean httpOnly)
92+
or
93+
this.getNumArgument() = 10 and this.getArgument(9).(BooleanLiteral).getBooleanValue() = true // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
94+
)
95+
}
10596
}
10697

107-
/**
108-
* Holds if `node` is a string that contains `httponly` and which flows to the second argument
109-
* of a method to set a cookie.
110-
*/
111-
predicate setHttpOnlyInSetCookie(DataFlow::Node node) {
112-
exists(SetCookieMethodAccess sa |
113-
hasHttpOnlyExpr(node.asExpr()) and
114-
DataFlow::localExprFlow(node.asExpr(), sa.getArgument(1))
115-
)
98+
/** The cookie constructor. */
99+
class CookieTaintPreservingConstructor extends Constructor, TaintPreservingCallable {
100+
CookieTaintPreservingConstructor() {
101+
this.getDeclaringType() instanceof CookieClass and
102+
not exists(HttpOnlyNewCookie hie | hie.getConstructor() = this)
103+
}
104+
105+
override predicate returnsTaintFrom(int arg) { arg = 0 }
116106
}
117107

118-
/** Holds if `cie` is an invocation of a JAX-RS `NewCookie` constructor that sets `HttpOnly` to true. */
119-
predicate setHttpOnlyInNewCookie(ClassInstanceExpr cie) {
120-
cie.getConstructedType().hasQualifiedName(["javax.ws.rs.core", "jakarta.ws.rs.core"], "NewCookie") and
121-
(
122-
cie.getNumArgument() = 6 and cie.getArgument(5).(BooleanLiteral).getBooleanValue() = true // NewCookie(Cookie cookie, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
123-
or
124-
cie.getNumArgument() = 8 and
125-
cie.getArgument(6).getType() instanceof BooleanType and
126-
cie.getArgument(7).(BooleanLiteral).getBooleanValue() = true // NewCookie(String name, String value, String path, String domain, String comment, int maxAge, boolean secure, boolean httpOnly)
127-
or
128-
cie.getNumArgument() = 10 and cie.getArgument(9).(BooleanLiteral).getBooleanValue() = true // NewCookie(String name, String value, String path, String domain, int version, String comment, int maxAge, Date expiry, boolean secure, boolean httpOnly)
129-
)
108+
/** The method call `toString` to get a stringified cookie representation. */
109+
class CookieInstanceExpr extends TaintPreservingCallable {
110+
CookieInstanceExpr() {
111+
this.getDeclaringType() instanceof CookieClass and
112+
this.hasName("toString")
113+
}
114+
115+
override predicate returnsTaintFrom(int arg) { arg = -1 }
130116
}
131117

132118
/**
@@ -163,15 +149,6 @@ class MissingHttpOnlyConfiguration extends TaintTracking::Configuration {
163149
override predicate isSink(DataFlow::Node sink) { sink instanceof CookieResponseSink }
164150

165151
override predicate isSanitizer(DataFlow::Node node) {
166-
// cookie.setHttpOnly(true)
167-
setHttpOnlyMethodAccess(node)
168-
or
169-
// response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure")
170-
setHttpOnlyInSetCookie(node)
171-
or
172-
// new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)
173-
setHttpOnlyInNewCookie(node.asExpr())
174-
or
175152
// Test class or method
176153
isTestMethod(node)
177154
}

0 commit comments

Comments
 (0)