Skip to content

Commit 3a5d711

Browse files
committed
Add cookie sinks
1 parent 435d1f9 commit 3a5d711

File tree

4 files changed

+82
-14
lines changed

4 files changed

+82
-14
lines changed
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/** Provides definitions related to the Netty framework. */
2+
3+
import java
4+
5+
/** The interface `Cookie` in the packages `io.netty.handler.codec.http` and `io.netty.handler.codec.http.cookie`. */
6+
class NettyCookie extends Interface {
7+
NettyCookie() { this.hasQualifiedName("io.netty.handler.codec.http" + [".cookie", ""], "Cookie") }
8+
}
9+
10+
/** The class `DefaultCookie` in the packages `io.netty.handler.codec.http` and `io.netty.handler.codec.http.cookie`. */
11+
class NettyDefaultCookie extends Class {
12+
NettyDefaultCookie() {
13+
this.hasQualifiedName("io.netty.handler.codec.http" + [".cookie", ""], "DefaultCookie")
14+
}
15+
}
16+
17+
/** The method `setValue` of the interface `Cookie` or a class implementing it. */
18+
class NettySetCookieValueMethod extends Method {
19+
NettySetCookieValueMethod() {
20+
this.getDeclaringType*() instanceof NettyCookie and
21+
this.hasName("setValue")
22+
}
23+
}

java/ql/lib/semmle/code/java/frameworks/Servlets.qll

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ class TypeCookie extends Class {
244244
}
245245

246246
/**
247-
* The method `getValue(String)` declared in `javax.servlet.http.Cookie`.
247+
* The method `getValue()` declared in `javax.servlet.http.Cookie`.
248248
*/
249249
class CookieGetValueMethod extends Method {
250250
CookieGetValueMethod() {
@@ -254,6 +254,16 @@ class CookieGetValueMethod extends Method {
254254
}
255255
}
256256

257+
/**
258+
* The method `setValue(String)` declared in `javax.servlet.http.Cookie`.
259+
*/
260+
class CookieSetValueMethod extends Method {
261+
CookieSetValueMethod() {
262+
this.getDeclaringType() instanceof TypeCookie and
263+
this.hasName("setValue")
264+
}
265+
}
266+
257267
/**
258268
* The method `getName()` declared in `javax.servlet.http.Cookie`.
259269
*/
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/** Provides definitions to reason about HTTP cookies. */
2+
3+
import java
4+
private import semmle.code.java.frameworks.Netty
5+
private import semmle.code.java.frameworks.Servlets
6+
7+
/** An expression setting the value of a cookie. */
8+
abstract class SetCookieValue extends Expr { }
9+
10+
private class ServletSetCookieValue extends SetCookieValue {
11+
ServletSetCookieValue() {
12+
exists(Call c |
13+
c.(ClassInstanceExpr).getConstructedType() instanceof TypeCookie and
14+
this = c.getArgument(1)
15+
or
16+
c.(MethodCall).getMethod() instanceof CookieSetValueMethod and
17+
this = c.getArgument(0)
18+
)
19+
}
20+
}
21+
22+
private class NettySetCookieValue extends SetCookieValue {
23+
NettySetCookieValue() {
24+
exists(Call c |
25+
c.(ClassInstanceExpr).getConstructedType() instanceof NettyDefaultCookie and
26+
this = c.getArgument(1)
27+
or
28+
c.(MethodCall).getMethod() instanceof NettySetCookieValueMethod and
29+
this = c.getArgument(0)
30+
)
31+
}
32+
}

java/ql/lib/semmle/code/java/security/InsecureRandomnessQuery.qll

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
import java
44
private import semmle.code.java.frameworks.OpenSaml
55
private import semmle.code.java.frameworks.Servlets
6-
private import semmle.code.java.security.SensitiveActions
7-
private import semmle.code.java.security.SensitiveApi
8-
private import semmle.code.java.dataflow.TaintTracking
96
private import semmle.code.java.dataflow.ExternalFlow
7+
private import semmle.code.java.dataflow.TaintTracking
8+
private import semmle.code.java.security.Cookies
109
private import semmle.code.java.security.RandomQuery
10+
private import semmle.code.java.security.SensitiveActions
11+
private import semmle.code.java.security.SensitiveApi
1112

1213
/**
1314
* A node representing a source of insecure randomness.
@@ -49,16 +50,7 @@ abstract class InsecureRandomnessSink extends DataFlow::Node { }
4950
* A node which sets the value of a cookie.
5051
*/
5152
private class CookieSink extends InsecureRandomnessSink {
52-
CookieSink() {
53-
exists(Call c |
54-
c.(ClassInstanceExpr).getConstructedType() instanceof TypeCookie and
55-
this.asExpr() = c.getArgument(1)
56-
or
57-
c.(MethodCall).getMethod().getDeclaringType() instanceof TypeCookie and
58-
c.(MethodCall).getMethod().hasName("setValue") and
59-
this.asExpr() = c.getArgument(0)
60-
)
61-
}
53+
CookieSink() { this.asExpr() instanceof SetCookieValue }
6254
}
6355

6456
private class SensitiveActionSink extends InsecureRandomnessSink {
@@ -89,6 +81,17 @@ module InsecureRandomnessConfig implements DataFlow::ConfigSig {
8981
n1.asExpr() = mc.getArgument(0) and
9082
n2.asExpr() = mc
9183
)
84+
or
85+
// TODO: Once we have a default sanitizer for UUIDs, we can convert these to global summaries.
86+
exists(Call c |
87+
c.(ClassInstanceExpr).getConstructedType().hasQualifiedName("java.util", "UUID") and
88+
n1.asExpr() = c.getAnArgument() and
89+
n2.asExpr() = c
90+
or
91+
c.(MethodCall).getMethod().hasQualifiedName("java.util", "UUID", "toString") and
92+
n1.asExpr() = c.getQualifier() and
93+
n2.asExpr() = c
94+
)
9295
}
9396
}
9497

0 commit comments

Comments
 (0)