Skip to content

Commit 897cd53

Browse files
atorralbaatorralba
committed
Created JWT.qll and refactored to use CSV models
1 parent 3e4ccaf commit 897cd53

File tree

3 files changed

+177
-186
lines changed

3 files changed

+177
-186
lines changed

‎java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql

Lines changed: 4 additions & 186 deletions
Original file line numberDiff line numberDiff line change
@@ -10,191 +10,9 @@
1010
*/
1111

1212
import java
13-
import semmle.code.java.dataflow.DataFlow
13+
import semmle.code.java.security.JWT
1414

15-
/** The interface `io.jsonwebtoken.JwtParser`. */
16-
class TypeJwtParser extends Interface {
17-
TypeJwtParser() { this.hasQualifiedName("io.jsonwebtoken", "JwtParser") }
18-
}
19-
20-
/** The interface `io.jsonwebtoken.JwtParser` or a type derived from it. */
21-
class TypeDerivedJwtParser extends RefType {
22-
TypeDerivedJwtParser() { this.getASourceSupertype*() instanceof TypeJwtParser }
23-
}
24-
25-
/** The interface `io.jsonwebtoken.JwtParserBuilder`. */
26-
class TypeJwtParserBuilder extends Interface {
27-
TypeJwtParserBuilder() { this.hasQualifiedName("io.jsonwebtoken", "JwtParserBuilder") }
28-
}
29-
30-
/** The interface `io.jsonwebtoken.JwtHandler`. */
31-
class TypeJwtHandler extends Interface {
32-
TypeJwtHandler() { this.hasQualifiedName("io.jsonwebtoken", "JwtHandler") }
33-
}
34-
35-
/** The class `io.jsonwebtoken.JwtHandlerAdapter`. */
36-
class TypeJwtHandlerAdapter extends Class {
37-
TypeJwtHandlerAdapter() { this.hasQualifiedName("io.jsonwebtoken", "JwtHandlerAdapter") }
38-
}
39-
40-
/** The `parse(token, handler)` method defined in `JwtParser`. */
41-
private class JwtParserParseHandlerMethod extends Method {
42-
JwtParserParseHandlerMethod() {
43-
this.hasName("parse") and
44-
this.getDeclaringType() instanceof TypeJwtParser and
45-
this.getNumberOfParameters() = 2
46-
}
47-
}
48-
49-
/** The `parse(token)`, `parseClaimsJwt(token)` and `parsePlaintextJwt(token)` methods defined in `JwtParser`. */
50-
private class JwtParserInsecureParseMethod extends Method {
51-
JwtParserInsecureParseMethod() {
52-
this.hasName(["parse", "parseClaimsJwt", "parsePlaintextJwt"]) and
53-
this.getNumberOfParameters() = 1 and
54-
this.getDeclaringType() instanceof TypeJwtParser
55-
}
56-
}
57-
58-
/** The `on(Claims|Plaintext)Jwt` methods defined in `JwtHandler`. */
59-
private class JwtHandlerOnJwtMethod extends Method {
60-
JwtHandlerOnJwtMethod() {
61-
this.hasName(["onClaimsJwt", "onPlaintextJwt"]) and
62-
this.getNumberOfParameters() = 1 and
63-
this.getDeclaringType() instanceof TypeJwtHandler
64-
}
65-
}
66-
67-
/** The `on(Claims|Plaintext)Jwt` methods defined in `JwtHandlerAdapter`. */
68-
private class JwtHandlerAdapterOnJwtMethod extends Method {
69-
JwtHandlerAdapterOnJwtMethod() {
70-
this.hasName(["onClaimsJwt", "onPlaintextJwt"]) and
71-
this.getNumberOfParameters() = 1 and
72-
this.getDeclaringType() instanceof TypeJwtHandlerAdapter
73-
}
74-
}
75-
76-
/**
77-
* Holds if `parseHandlerExpr` is an insecure `JwtHandler`.
78-
* That is, it overrides a method from `JwtHandlerOnJwtMethod` and the override is not defined on `JwtHandlerAdapter`.
79-
* `JwtHandlerAdapter`'s overrides are safe since they always throw an exception.
80-
*/
81-
private predicate isInsecureJwtHandler(Expr parseHandlerExpr) {
82-
exists(RefType t |
83-
parseHandlerExpr.getType() = t and
84-
t.getASourceSupertype*() instanceof TypeJwtHandler and
85-
exists(Method m |
86-
m = t.getAMethod() and
87-
m.getASourceOverriddenMethod+() instanceof JwtHandlerOnJwtMethod and
88-
not m.getSourceDeclaration() instanceof JwtHandlerAdapterOnJwtMethod
89-
)
90-
)
91-
}
92-
93-
/**
94-
* An access to an insecure parsing method.
95-
* That is, either a call to a `parse(token)`, `parseClaimsJwt(token)` or `parsePlaintextJwt(token)` method or
96-
* a call to a `parse(token, handler)` method where the `handler` is considered insecure.
97-
*/
98-
private class JwtParserInsecureParseMethodAccess extends MethodAccess {
99-
JwtParserInsecureParseMethodAccess() {
100-
this.getMethod().getASourceOverriddenMethod*() instanceof JwtParserInsecureParseMethod
101-
or
102-
this.getMethod().getASourceOverriddenMethod*() instanceof JwtParserParseHandlerMethod and
103-
isInsecureJwtHandler(this.getArgument(1))
104-
}
105-
}
106-
107-
/**
108-
* Holds if `signingMa` directly or indirectly sets a signing key for `expr`, which is a `JwtParser`.
109-
* The `setSigningKey` and `setSigningKeyResolver` methods set a signing key for a `JwtParser`.
110-
* Directly means code like this:
111-
* ```java
112-
* Jwts.parser().setSigningKey(key).parse(token);
113-
* ```
114-
* Here the signing key is set directly on a `JwtParser`.
115-
* Indirectly means code like this:
116-
* ```java
117-
* Jwts.parserBuilder().setSigningKey(key).build().parse(token);
118-
* ```
119-
* In this case, the signing key is set on a `JwtParserBuilder` indirectly setting the key of `JwtParser` that is created by the call to `build`.
120-
*/
121-
private predicate isSigningKeySetter(Expr expr, MethodAccess signingMa) {
122-
any(SigningToInsecureMethodAccessDataFlow s)
123-
.hasFlow(DataFlow::exprNode(signingMa), DataFlow::exprNode(expr))
124-
}
125-
126-
/**
127-
* An expr that is a (sub-type of) `JwtParser` for which a signing key has been set and which is used as
128-
* the qualifier to a `JwtParserInsecureParseMethodAccess`.
129-
*/
130-
private class JwtParserWithSigningKeyExpr extends Expr {
131-
MethodAccess signingMa;
132-
133-
JwtParserWithSigningKeyExpr() {
134-
this.getType() instanceof TypeDerivedJwtParser and
135-
isSigningKeySetter(this, signingMa)
136-
}
137-
138-
/** Gets the method access that sets the signing key for this parser. */
139-
MethodAccess getSigningMethodAccess() { result = signingMa }
140-
}
141-
142-
/**
143-
* Models flow from `SigningKeyMethodAccess`es to qualifiers of `JwtParserInsecureParseMethodAccess`es.
144-
* This is used to determine whether a `JwtParser` has a signing key set.
145-
*/
146-
private class SigningToInsecureMethodAccessDataFlow extends DataFlow::Configuration {
147-
SigningToInsecureMethodAccessDataFlow() { this = "SigningToExprDataFlow" }
148-
149-
override predicate isSource(DataFlow::Node source) {
150-
source.asExpr() instanceof SigningKeyMethodAccess
151-
}
152-
153-
override predicate isSink(DataFlow::Node sink) {
154-
any(JwtParserInsecureParseMethodAccess ma).getQualifier() = sink.asExpr()
155-
}
156-
157-
/** Models the builder style of `JwtParser` and `JwtParserBuilder`. */
158-
override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
159-
(
160-
pred.asExpr().getType() instanceof TypeDerivedJwtParser or
161-
pred.asExpr().getType().(RefType).getASourceSupertype*() instanceof TypeJwtParserBuilder
162-
) and
163-
succ.asExpr().(MethodAccess).getQualifier() = pred.asExpr()
164-
}
165-
}
166-
167-
/** An access to the `setSigningKey` or `setSigningKeyResolver` method (or an overridden method) defined in `JwtParser` and `JwtParserBuilder`. */
168-
private class SigningKeyMethodAccess extends MethodAccess {
169-
SigningKeyMethodAccess() {
170-
exists(Method m |
171-
m.hasName(["setSigningKey", "setSigningKeyResolver"]) and
172-
m.getNumberOfParameters() = 1 and
173-
(
174-
m.getDeclaringType() instanceof TypeJwtParser or
175-
m.getDeclaringType() instanceof TypeJwtParserBuilder
176-
)
177-
|
178-
m = this.getMethod().getASourceOverriddenMethod*()
179-
)
180-
}
181-
}
182-
183-
/**
184-
* Holds if the `MethodAccess` `ma` occurs in a test file. A test file is any file that
185-
* is a direct or indirect child of a directory named `test`, ignoring case.
186-
*/
187-
private predicate isInTestFile(MethodAccess ma) {
188-
exists(string lowerCasedAbsolutePath |
189-
lowerCasedAbsolutePath = ma.getLocation().getFile().getAbsolutePath().toLowerCase()
190-
|
191-
lowerCasedAbsolutePath.matches("%/test/%") and
192-
not lowerCasedAbsolutePath
193-
.matches("%/ql/test/query-tests/security/CWE-347/%".toLowerCase())
194-
)
195-
}
196-
197-
from JwtParserInsecureParseMethodAccess ma, JwtParserWithSigningKeyExpr parserExpr
198-
where ma.getQualifier() = parserExpr and not isInTestFile(ma)
199-
select ma, "A signing key is set $@, but the signature is not verified.",
15+
from JwtParserWithInsecureParseSink sink, JwtParserWithSigningKeyExpr parserExpr
16+
where sink.asExpr() = parserExpr
17+
select sink.getParseMethodAccess(), "A signing key is set $@, but the signature is not verified.",
20018
parserExpr.getSigningMethodAccess(), "here"

‎java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ private module Frameworks {
8181
private import semmle.code.java.security.XSS
8282
private import semmle.code.java.security.LdapInjection
8383
private import semmle.code.java.security.XPath
84+
private import semmle.code.java.security.JWT
8485
}
8586

8687
private predicate sourceModelCsv(string row) {

‎java/ql/src/semmle/code/java/security/JWT.qll

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/** Provides classes for working with JWT libraries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.DataFlow
5+
import semmle.code.java.dataflow.ExternalFlow
6+
7+
/**
8+
* An expr that is a (sub-type of) `JwtParser` for which a signing key has been set
9+
*/
10+
class JwtParserWithSigningKeyExpr extends Expr {
11+
MethodAccess signingMa;
12+
13+
JwtParserWithSigningKeyExpr() { isSigningKeySetter(this, signingMa) }
14+
15+
/** Gets the method access that sets the signing key for this parser. */
16+
MethodAccess getSigningMethodAccess() { result = signingMa }
17+
}
18+
19+
/**
20+
* The qualifier of an insecure parsing method.
21+
* That is, either the qualifier of a call to a `parse(token)`, `parseClaimsJwt(token)` or `parsePlaintextJwt(token)` method or
22+
* the qualifier of a call to a `parse(token, handler)` method where the `handler` is considered insecure.
23+
*/
24+
class JwtParserWithInsecureParseSink extends DataFlow::Node {
25+
MethodAccess insecureParseMa;
26+
27+
JwtParserWithInsecureParseSink() {
28+
insecureParseMa.getQualifier() = this.asExpr() and
29+
(
30+
sinkNode(this, "jwt-insecure-parse")
31+
or
32+
sinkNode(this, "jwt-insecure-parse-handler") and
33+
isInsecureJwtHandler(insecureParseMa.getArgument(1))
34+
)
35+
}
36+
37+
MethodAccess getParseMethodAccess() { result = insecureParseMa }
38+
}
39+
40+
/**
41+
* Holds if `signingMa` directly or indirectly sets a signing key for `expr`, which is a `JwtParser`.
42+
* The `setSigningKey` and `setSigningKeyResolver` methods set a signing key for a `JwtParser`.
43+
*
44+
* Directly means code like this (the signing key is set directly on a `JwtParser`):
45+
* ```java
46+
* Jwts.parser().setSigningKey(key).parse(token);
47+
* ```
48+
*
49+
* Indirectly means code like this (the signing key is set on a `JwtParserBuilder` indirectly setting the key of `JwtParser` that is created by the call to `build`):
50+
* ```java
51+
* Jwts.parserBuilder().setSigningKey(key).build().parse(token);
52+
* ```
53+
*/
54+
private predicate isSigningKeySetter(Expr expr, MethodAccess signingMa) {
55+
any(SigningToInsecureMethodAccessDataFlow s)
56+
.hasFlow(DataFlow::exprNode(signingMa), DataFlow::exprNode(expr))
57+
}
58+
59+
/**
60+
* Holds if `parseHandlerExpr` is an insecure `JwtHandler`.
61+
* That is, it overrides a method from `JwtHandlerOnJwtMethod` and the override is not defined on `JwtHandlerAdapter`.
62+
* `JwtHandlerAdapter`'s overrides are safe since they always throw an exception.
63+
*/
64+
private predicate isInsecureJwtHandler(Expr parseHandlerExpr) {
65+
exists(RefType t |
66+
parseHandlerExpr.getType() = t and
67+
t.getASourceSupertype*() instanceof TypeJwtHandler and
68+
exists(Method m |
69+
m = t.getAMethod() and
70+
m.getASourceOverriddenMethod+() instanceof JwtHandlerOnJwtMethod and
71+
not m.getSourceDeclaration() instanceof JwtHandlerAdapterOnJwtMethod
72+
)
73+
)
74+
}
75+
76+
/** CSV source models representing methods that assign signing keys to a JWT parser. */
77+
private class SigningKeySourceModel extends SourceModelCsv {
78+
override predicate row(string row) {
79+
row =
80+
[
81+
"io.jsonwebtoken;JwtParser;true;setSigningKey;;;ReturnValue;jwt-signing-key",
82+
"io.jsonwebtoken;JwtParser;true;setSigningKeyResolver;;;ReturnValue;jwt-signing-key",
83+
"io.jsonwebtoken;JwtParserBuilder;true;setSigningKey;;;ReturnValue;jwt-signing-key",
84+
"io.jsonwebtoken;JwtParserBuilder;true;setSigningKeyResolver;;;ReturnValue;jwt-signing-key"
85+
]
86+
}
87+
}
88+
89+
/** CSV sink models representing qualifiers of methods that insecurely parse a JWT */
90+
private class InsecureJwtParseSinkModel extends SinkModelCsv {
91+
override predicate row(string row) {
92+
row =
93+
[
94+
"io.jsonwebtoken;JwtParser;true;parse;(String);;Argument[-1];jwt-insecure-parse",
95+
"io.jsonwebtoken;JwtParser;true;parseClaimsJwt;;;Argument[-1];jwt-insecure-parse",
96+
"io.jsonwebtoken;JwtParser;true;parsePlaintextJwt;;;Argument[-1];jwt-insecure-parse"
97+
]
98+
}
99+
}
100+
101+
/** CSV sink models representing qualifiers of methods that insecurely parse a JWT with a handler */
102+
private class InsecureJwtParseHandlerSinkModel extends SinkModelCsv {
103+
override predicate row(string row) {
104+
row =
105+
[
106+
"io.jsonwebtoken;JwtParser;true;parse;(String,JwtHandler<T>);;Argument[-1];jwt-insecure-parse-handler"
107+
]
108+
}
109+
}
110+
111+
/** Models the builder style of `JwtParser` and `JwtParserBuilder`. */
112+
private predicate jwtParserStep(Expr parser, MethodAccess ma) {
113+
(
114+
parser.getType().(RefType).getASourceSupertype*() instanceof TypeJwtParser or
115+
parser.getType().(RefType).getASourceSupertype*() instanceof TypeJwtParserBuilder
116+
) and
117+
ma.getQualifier() = parser
118+
}
119+
120+
/**
121+
* Models flow from signing keys assignements to qualifiers of JWT insecure parsers.
122+
* This is used to determine whether a `JwtParser` has a signing key set.
123+
*/
124+
private class SigningToInsecureMethodAccessDataFlow extends DataFlow::Configuration {
125+
SigningToInsecureMethodAccessDataFlow() { this = "SigningToExprDataFlow" }
126+
127+
override predicate isSource(DataFlow::Node source) { sourceNode(source, "jwt-signing-key") }
128+
129+
override predicate isSink(DataFlow::Node sink) { sink instanceof JwtParserWithInsecureParseSink }
130+
131+
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
132+
jwtParserStep(node1.asExpr(), node2.asExpr())
133+
}
134+
}
135+
136+
/** The interface `io.jsonwebtoken.JwtParser`. */
137+
private class TypeJwtParser extends Interface {
138+
TypeJwtParser() { this.hasQualifiedName("io.jsonwebtoken", "JwtParser") }
139+
}
140+
141+
/** The interface `io.jsonwebtoken.JwtParserBuilder`. */
142+
private class TypeJwtParserBuilder extends Interface {
143+
TypeJwtParserBuilder() { this.hasQualifiedName("io.jsonwebtoken", "JwtParserBuilder") }
144+
}
145+
146+
/** The interface `io.jsonwebtoken.JwtHandler`. */
147+
private class TypeJwtHandler extends Interface {
148+
TypeJwtHandler() { this.hasQualifiedName("io.jsonwebtoken", "JwtHandler") }
149+
}
150+
151+
/** The class `io.jsonwebtoken.JwtHandlerAdapter`. */
152+
private class TypeJwtHandlerAdapter extends Class {
153+
TypeJwtHandlerAdapter() { this.hasQualifiedName("io.jsonwebtoken", "JwtHandlerAdapter") }
154+
}
155+
156+
/** The `on(Claims|Plaintext)Jwt` methods defined in `JwtHandler`. */
157+
private class JwtHandlerOnJwtMethod extends Method {
158+
JwtHandlerOnJwtMethod() {
159+
this.hasName(["onClaimsJwt", "onPlaintextJwt"]) and
160+
this.getNumberOfParameters() = 1 and
161+
this.getDeclaringType() instanceof TypeJwtHandler
162+
}
163+
}
164+
165+
/** The `on(Claims|Plaintext)Jwt` methods defined in `JwtHandlerAdapter`. */
166+
private class JwtHandlerAdapterOnJwtMethod extends Method {
167+
JwtHandlerAdapterOnJwtMethod() {
168+
this.hasName(["onClaimsJwt", "onPlaintextJwt"]) and
169+
this.getNumberOfParameters() = 1 and
170+
this.getDeclaringType() instanceof TypeJwtHandlerAdapter
171+
}
172+
}

0 commit comments

Comments
 (0)