Skip to content

Commit 2a30898

Browse files
atorralbaatorralba
committed
Go: Promote go/missing-jwt-signature-check from experimental
1 parent 99ac640 commit 2a30898

26 files changed

+332
-141
lines changed

‎go/ql/lib/ext/github.com.dgrijalva.jwt-go.model.yml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,19 @@ extensions:
33
pack: codeql/go-all
44
extensible: sinkModel
55
data:
6-
- ["github.com/dgrijalva/jwt-go", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
76
- ["github.com/dgrijalva/jwt-go", "SigningMethod", True, "Sign", "", "", "Argument[1]", "credentials-key", "manual"]
7+
- ["github.com/dgrijalva/jwt-go", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
8+
- ["github.com/dgrijalva/jwt-go", "Parser", True, "ParseUnverified", "", "", "Argument[0]", "jwt", "manual"]
9+
- addsTo:
10+
pack: codeql/go-all
11+
extensible: summaryModel
12+
data:
13+
- ["github.com/dgrijalva/jwt-go", "", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
14+
- ["github.com/dgrijalva/jwt-go", "Parser", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
15+
- ["github.com/dgrijalva/jwt-go", "", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
16+
- ["github.com/dgrijalva/jwt-go", "Parser", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
17+
- ["github.com/dgrijalva/jwt-go", "", True, "ParseECPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
18+
- ["github.com/dgrijalva/jwt-go", "", True, "ParseECPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
19+
- ["github.com/dgrijalva/jwt-go", "", True, "ParseRSAPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
20+
- ["github.com/dgrijalva/jwt-go", "", True, "ParseRSAPrivateKeyFromPEMWithPassword", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
21+
- ["github.com/dgrijalva/jwt-go", "", True, ParseRSAPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]

‎go/ql/lib/ext/github.com.go-jose.go-jose.model.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/go-all
4+
extensible: sinkModel
5+
data:
6+
- ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "JSONWebToken", True, "UnsafeClaimsWithoutVerification", "", "", "Argument[-1]", "jwt", "manual"]
7+
- addsTo:
8+
pack: codeql/go-all
9+
extensible: summaryModel
10+
data:
11+
- ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "", True, "ParseEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
12+
- ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "", True, "ParseSigned", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
13+
- ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "NestedJSONWebToken", True, "ParseSignedAndEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
14+
- ["github.com/go-jose/go-jose/$ANYVERSION/jwt", "NestedJSONWebToken", True, "Decrypt", "", "", "Argument[-1]", "ReturnValue[0]", "taint", "manual"]

‎go/ql/lib/ext/github.com.golang-jwt.jwt.model.yml

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,21 @@ extensions:
33
pack: codeql/go-all
44
extensible: sinkModel
55
data:
6-
- ["github.com/golang-jwt/jwt", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
76
- ["github.com/golang-jwt/jwt", "SigningMethod", True, "Sign", "", "", "Argument[1]", "credentials-key", "manual"]
7+
- ["github.com/golang-jwt/jwt", "Token", True, "SignedString", "", "", "Argument[0]", "credentials-key", "manual"]
8+
- ["github.com/golang-jwt/jwt", "Parser", True, "ParseUnverified", "", "", "Argument[0]", "jwt", "manual"]
9+
- addsTo:
10+
pack: codeql/go-all
11+
extensible: summaryModel
12+
data:
13+
- ["github.com/golang-jwt/jwt", "", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
14+
- ["github.com/golang-jwt/jwt", "Parser", True, "Parse", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
15+
- ["github.com/golang-jwt/jwt", "", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
16+
- ["github.com/golang-jwt/jwt", "Parser", True, "ParseWithClaims", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
17+
- ["github.com/golang-jwt/jwt", "", True, "ParseECPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
18+
- ["github.com/golang-jwt/jwt", "", True, "ParseECPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
19+
- ["github.com/golang-jwt/jwt", "", True, "ParseEdPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
20+
- ["github.com/golang-jwt/jwt", "", True, "ParseEdPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
21+
- ["github.com/golang-jwt/jwt", "", True, "ParseRSAPrivateKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
22+
- ["github.com/golang-jwt/jwt", "", True, "ParseRSAPublicKeyFromPEM", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
23+
- ["github.com/golang-jwt/jwt", "", True, "RegisterSigningMethod", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]

‎go/ql/lib/ext/gopkg.in.square.go-jose.model.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/go-all
4+
extensible: sinkModel
5+
data:
6+
- ["gopkg.in/square/go-jose/$ANYVERSION/jwt", "JSONWebToken", True, "UnsafeClaimsWithoutVerification", "", "", "Argument[-1]", "jwt", "manual"]
7+
- addsTo:
8+
pack: codeql/go-all
9+
extensible: summaryModel
10+
data:
11+
- ["gopkg.in/square/go-jose/$ANYVERSION/jwt", "", True, "ParseEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
12+
- ["gopkg.in/square/go-jose/$ANYVERSION/jwt", "", True, "ParseSigned", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
13+
- ["gopkg.in/square/go-jose/$ANYVERSION/jwt", "NestedJSONWebToken", True, "ParseSignedAndEncrypted", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
14+
- ["gopkg.in/square/go-jose/$ANYVERSION/jwt", "NestedJSONWebToken", True, "Decrypt", "", "", "Argument[-1]", "ReturnValue[0]", "taint", "manual"]

‎go/ql/lib/go.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import semmle.go.frameworks.GoMicro
5252
import semmle.go.frameworks.GoRestfulHttp
5353
import semmle.go.frameworks.Gqlgen
5454
import semmle.go.frameworks.Iris
55+
import semmle.go.frameworks.Jwt
5556
import semmle.go.frameworks.K8sIoApimachineryPkgRuntime
5657
import semmle.go.frameworks.K8sIoApiCoreV1
5758
import semmle.go.frameworks.K8sIoClientGo

‎go/ql/lib/semmle/go/frameworks/GoJose.qll

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,40 @@ private import semmle.go.security.HardcodedCredentials
99
private module GoJose {
1010
private class GoJoseKey extends HardcodedCredentials::Sink {
1111
GoJoseKey() {
12-
exists(Field f, string pkg |
13-
pkg =
14-
[
15-
package("github.com/square/go-jose", ""), package("github.com/go-jose/go-jose", ""),
16-
"gopkg.in/square/go-jose.v2"
17-
]
18-
|
19-
f.hasQualifiedName(pkg, ["Recipient", "SigningKey"], "Key") and
12+
exists(Field f |
13+
f.hasQualifiedName(goJosePackage(), ["Recipient", "SigningKey"], "Key") and
2014
f.getAWrite().getRhs() = this
2115
)
2216
}
2317
}
18+
19+
private string goJosePackage() {
20+
result =
21+
[
22+
package("github.com/square/go-jose", ""), package("github.com/go-jose/go-jose", ""),
23+
"gopkg.in/square/go-jose.v2"
24+
]
25+
}
26+
27+
/**
28+
* Provides classes and predicates for working with the `gopkg.in/square/go-jose/jwt` and
29+
* `github.com/go-jose/go-jose/jwt` packages.
30+
*/
31+
private module Jwt {
32+
private import semmle.go.security.MissingJwtSignatureCheckCustomizations::MissingJwtSignatureCheck
33+
34+
/** The method `JSONWebToken.Claims`. */
35+
private class GoJoseParseWithClaims extends JwtSafeParse {
36+
GoJoseParseWithClaims() {
37+
this.(Method).hasQualifiedName(goJoseJwtPackage(), "JSONWebToken", "Claims")
38+
}
39+
40+
override int getTokenArgNum() { result = -1 }
41+
}
42+
43+
/** Gets the package names `gopkg.in/square/go-jose/jwt` and `github.com/go-jose/go-jose/jwt`. */
44+
private string goJoseJwtPackage() {
45+
result = package(["gopkg.in/square/go-jose", "github.com/go-jose/go-jose"], "jwt")
46+
}
47+
}
2448
}

‎go/ql/lib/semmle/go/frameworks/Jwt.qll

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* Provides classes and predicates for working with the `github.com/golang-jwt/jwt` and
3+
* `github.com/dgrijalva/jwt-go` packages.
4+
*/
5+
6+
import go
7+
private import semmle.go.security.MissingJwtSignatureCheckCustomizations::MissingJwtSignatureCheck
8+
9+
/** The function `jwt.Parse` or the method `Parser.Parse`. */
10+
private class GolangJwtParse extends JwtSafeParse {
11+
GolangJwtParse() {
12+
this.hasQualifiedName(golangJwtPackage(), "Parse")
13+
or
14+
this.(Method).hasQualifiedName(golangJwtPackage(), "Parser", "Parse")
15+
}
16+
17+
override int getTokenArgNum() { result = 0 }
18+
}
19+
20+
/** The function `jwt.ParseWithClaims` or the method `Parser.ParseWithClaims`. */
21+
private class GolangJwtParseWithClaims extends JwtSafeParse {
22+
GolangJwtParseWithClaims() {
23+
this.hasQualifiedName(golangJwtPackage(), "ParseWithClaims")
24+
or
25+
this.(Method).hasQualifiedName(golangJwtPackage(), "Parser", "ParseWithClaims")
26+
}
27+
28+
override int getTokenArgNum() { result = 0 }
29+
}
30+
31+
/** The function `jwt.ParseFromRequest`. */
32+
private class GolangJwtParseFromRequest extends JwtSafeParse {
33+
GolangJwtParseFromRequest() {
34+
this.hasQualifiedName(golangJwtRequestPackage(), "ParseFromRequest")
35+
}
36+
37+
override int getTokenArgNum() { result = 0 }
38+
}
39+
40+
/** The function `jwt.ParseFromRequestWithClaims`. */
41+
private class GolangJwtParseFromRequestWithClaims extends JwtSafeParse {
42+
GolangJwtParseFromRequestWithClaims() {
43+
this.hasQualifiedName(golangJwtRequestPackage(), "ParseFromRequestWithClaims")
44+
}
45+
46+
override int getTokenArgNum() { result = 0 }
47+
}
48+
49+
/** Gets the pakcage names `github.com/golang-jwt/jwt` and `github.com/dgrijalva/jwt-go`. */
50+
private string golangJwtPackage() {
51+
result = package(["github.com/golang-jwt/jwt", "github.com/dgrijalva/jwt-go"], "")
52+
}
53+
54+
/** Gets the package names `github.com/golang-jwt/jwt/request` and `github.com/dgrijalva/jwt-go/request`. */
55+
private string golangJwtRequestPackage() {
56+
result = package(["github.com/golang-jwt/jwt", "github.com/dgrijalva/jwt-go"], "request")
57+
}

‎go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Provides a taint tracking flow for reasoning about JWT vulnerabilities.
3+
*
4+
* Note: for performance reasons, only import this file if `MissingJwtSignatureCheck::Config` or `MissingJwtSignatureCheck::Flow` are needed,
5+
* otherwise `MissingJwtSignatureCheckCustomizations` should be imported instead.
6+
*/
7+
8+
import go
9+
10+
/** Provides a taint-tracking flow for reasoning about JWT vulnerabilities. */
11+
module MissingJwtSignatureCheck {
12+
import MissingJwtSignatureCheckCustomizations::MissingJwtSignatureCheck
13+
14+
module Config implements DataFlow::ConfigSig {
15+
predicate isSource(DataFlow::Node source) {
16+
source instanceof Source and
17+
not SafeParse::flow(source, _)
18+
}
19+
20+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
21+
22+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
23+
any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
24+
}
25+
}
26+
27+
module Flow = TaintTracking::Global<Config>;
28+
29+
private module SafeParseConfig implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node source) { source instanceof Source }
31+
32+
predicate isSink(DataFlow::Node sink) { sink = any(JwtSafeParse jwtParse).getTokenArg() }
33+
34+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
35+
any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
36+
}
37+
}
38+
39+
private module SafeParse = TaintTracking::Global<SafeParseConfig>;
40+
}

‎go/ql/lib/semmle/go/security/MissingJwtSignatureCheckCustomizations.qll

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import go
2+
private import semmle.go.dataflow.ExternalFlow
3+
private import codeql.util.Unit
4+
5+
module MissingJwtSignatureCheck {
6+
/**
7+
* A data flow source for JWT vulnerabilities.
8+
*/
9+
abstract class Source extends DataFlow::Node { }
10+
11+
/**
12+
* A data flow sink for JWT vulnerabilities.
13+
*/
14+
abstract class Sink extends DataFlow::Node { }
15+
16+
/**
17+
* A sanitizer for JWT vulnerabilities.
18+
*/
19+
abstract class Sanitizer extends DataFlow::Node { }
20+
21+
/** An additional flow step for JWT vulnerabilities. */
22+
class AdditionalFlowStep extends Unit {
23+
/**
24+
* Holds if the step from `node1` to `node2` should be considered a flow
25+
* step for configurations related to JWT vulnerabilities.
26+
*/
27+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
28+
}
29+
30+
/** A function that parses and correctly validates a JWT token. */
31+
abstract class JwtSafeParse extends Function {
32+
/** Gets the position of the JWT argument in a call to this function. */
33+
abstract int getTokenArgNum();
34+
35+
/** Gets the JWT argument of a call to this function. */
36+
DataFlow::Node getTokenArg() {
37+
this.getTokenArgNum() != -1 and result = this.getACall().getArgument(this.getTokenArgNum())
38+
or
39+
this.getTokenArgNum() = -1 and result = this.getACall().getReceiver()
40+
}
41+
}
42+
43+
private class DefaultSource extends Source instanceof UntrustedFlowSource { }
44+
45+
private class DefaultSink extends Sink {
46+
DefaultSink() { sinkNode(this, "jwt") }
47+
}
48+
}

‎go/ql/src/Security/CWE-347/MissingJwtSignatureCheck.qhelp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>Applications decoding a JSON Web Token (JWT) may be vulnerable when the
5+
signature is not correctly verified in the process.</p>
6+
</overview>
7+
<recommendation>
8+
<p>Always verify the signature by using the appropriate methods depending on the JWT library,
9+
or use a library that verifies it by default.</p>
10+
<recommendation>
11+
<example>
12+
<p>The following example shows a case where a JWT is parsed without verifying the
13+
signature.</p>
14+
<sample src="MissingJwtSignatureCheckBad.go" />
15+
<p>In the example below, the appropriate function for parsing a JWT
16+
and verifying its signature is used.</p>
17+
<sample src="MissingJwtSignatureCheckGood.go" />
18+
</example>
19+
<references>
20+
<li>
21+
22+
</li>
23+
</references>
24+
25+
</qhelp>

0 commit comments

Comments
 (0)