Skip to content

Go: Add JWT Algorithm Confusion and JWT decoding without Signature Verification #14081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Kwstubbs wants to merge 11 commits into from
Closed

Go: Add JWT Algorithm Confusion and JWT decoding without Signature Verification #14081

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4f2d15e
Initial JWT Library
Kwstubbs Aug 25, 2023
fafdee5
Finish
Kwstubbs Aug 29, 2023
7730ad6
Finish
Kwstubbs Aug 29, 2023
8d2bc38
removed user files
Kwstubbs Aug 29, 2023
01a28eb
update docs
Kwstubbs Sep 5, 2023
1d6a805
Spelling
Kwstubbs Sep 5, 2023
ecbde43
Formatting and errors
Kwstubbs Sep 14, 2023
3a2b66e
change note
Kwstubbs Sep 14, 2023
1370bac
Big upgrade
Kwstubbs Sep 14, 2023
21fa77d
grammar
Kwstubbs Sep 14, 2023
84359fe
Finish
Kwstubbs Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Formatting and errors
  • Loading branch information
@Kwstubbs
Kwstubbs committed Sep 14, 2023
commit ecbde43663c064d15c9be852b763a9958d9a768b
117 changes: 65 additions & 52 deletions go/ql/lib/semmle/go/frameworks/JWT.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,74 +1,87 @@
/** Provides models of commonly used functions in common JWT packages. */

import go

/**
* Provides models of commonly used functions in common JWT packages.
*/
module JWT {
string packageLestrrat() { result = package("github.com/lestrrat-go/jwx/v2/jwt", "") }
Comment thread
Kwstubbs marked this conversation as resolved.

string packageLestrrat() {
result =
package("github.com/lestrrat-go/jwx/v2/jwt", "")
string packageLestrratv1() { result = package("github.com/lestrrat-go/jwx/jwt", "") }
Comment thread
Kwstubbs marked this conversation as resolved.

}
string packageLestrratv1() {
result =
package("github.com/lestrrat-go/jwx/jwt", "")
}
string packagePathModern() {
result =
package(["github.com/golang-jwt/jwt/v5", "github.com/golang-jwt/jwt/v4"], "")
string packagePathModern() {
result = package(["github.com/golang-jwt/jwt/v5", "github.com/golang-jwt/jwt/v4"], "")
Comment thread
Kwstubbs marked this conversation as resolved.
}
string packagePathOld(){
result =
package(
"github.com/golang-jwt/jwt", "")
}


string packagePathOld() { result = package("github.com/golang-jwt/jwt", "") }

class NewParser extends Function {
NewParser() {
this.hasQualifiedName([packagePathModern()], "NewParser")
}
/**
* Call in golang-jwt to create new parser.
*/
NewParser() { this.hasQualifiedName(packagePathModern(), "NewParser") }
}

class WithValidMethods extends Function{
WithValidMethods() {
this.hasQualifiedName([packagePathModern()], "WithValidMethods")
}

class WithValidMethods extends Function {
/**
* Call to WithValidMethods in golang-jwt to specify allowed algorithms.
*/
WithValidMethods() { this.hasQualifiedName(packagePathModern(), "WithValidMethods") }
}

class SafeJWTParserMethod extends Method {
SafeJWTParserMethod() {

class SafeJwtParserMethod extends Method {
/**
* Methods to parse token that check token signature.
*/
SafeJwtParserMethod() {
this.hasQualifiedName(packagePathModern(), "Parser", ["Parse", "ParseWithClaims"])
}
}

class SafeJWTParserFunc extends Function{
SafeJWTParserFunc(){
this.hasQualifiedName([packagePathModern(),packagePathOld()], ["Parse", "ParseWithClaims"])

class SafeJwtParserFunc extends Function {
/**
* Functions to parse token that check token signature.
*/
SafeJwtParserFunc() {
this.hasQualifiedName([packagePathModern(), packagePathOld()], ["Parse", "ParseWithClaims"])
}
}

class UnafeJWTParserMethod extends Method{
UnafeJWTParserMethod(){
this.hasQualifiedName(packagePathModern(), "Parser", "ParseUnverified")

class UnafeJwtParserMethod extends Method {
Comment thread
Kwstubbs marked this conversation as resolved.
/**
* Methods to parse token that don't token signature.
*/
UnafeJwtParserMethod() {
this.hasQualifiedName(packagePathModern(), "Parser", "ParseUnverified")
}
}
class LestrratParse extends Function{
LestrratParse() {
this.hasQualifiedName(packageLestrrat(), "Parse")
}

class LestrratParse extends Function {
/**
* v2 Function to parse token that check token signature.
*/
LestrratParse() { this.hasQualifiedName(packageLestrrat(), "Parse") }
}
Comment on lines +64 to +69
Copy link
Copy Markdown
Contributor

@owen-mc owen-mc Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This model isn't used anywhere. Did you mean to use it somewhere? Or did you model it for completeness? Consider deleting - the model can easily be added in future if it is needed.

class LestrratParsev1 extends Function{
LestrratParsev1() {
this.hasQualifiedName(packageLestrratv1(), "Parse")
}

class LestrratParsev1 extends Function {
/**
* v1 Function to parse token that check token signature.
*/
LestrratParsev1() { this.hasQualifiedName(packageLestrratv1(), "Parse") }
}

class LestrratVerify extends Function {
LestrratVerify() {
this.hasQualifiedName(packageLestrratv1(), "WithVerify")
}
}
class LestrratParseInsecure extends Function{
LestrratParseInsecure() {
this.hasQualifiedName(packageLestrrat(), "ParseInsecure")
}
/**
* Funciton included as parse option to verify token.
*/
LestrratVerify() { this.hasQualifiedName(packageLestrratv1(), "WithVerify") }
}

class LestrratParseInsecure extends Function {
/**
* Funciton to parse token that don't check signature.
*/
LestrratParseInsecure() { this.hasQualifiedName(packageLestrrat(), "ParseInsecure") }
}
}
44 changes: 26 additions & 18 deletions go/ql/src/experimental/CWE-347/JWTParsingAlgorithm.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,37 @@
* @problem.severity error
* @security-severity 8.0
* @precision medium
* @id go/jwt-pitfalls
* @id go/jwt-alg-confusion
* @tags security
* external/cwe/cwe-347
*/

import go
import JWT
import DataFlow

from CallNode c, NewParser m, WithValidMethods wvm, Function func
where
(c.getTarget() = func and
// //Flow from NewParser to Parse (check that this call to Parse does not use a Parser that sets Valid Methods)
((func instanceof SafeJWTParserMethod and not exists( CallNode c2 | c2.getTarget() = m
and ( c2.getCall().getAnArgument() = wvm.getACall().asExpr()
and DataFlow::localFlow(c2.getAResult(), c.getReceiver()))
))
//ParserFunc creates a new default Parser on call that accepts all methods
or func instanceof SafeJWTParserFunc
)
//Check that the Parse(function or method) does not check the Token Method field, which most likely is a check for method type
and not exists(Field f |
f.hasQualifiedName(packagePathModern(), "Token", "Method")
and f.getARead().getRoot() = c.getCall().getAnArgument()
))

from CallNode c, Function func
where
(
c.getTarget() = func and
// //Flow from NewParser to Parse (check that this call to Parse does not use a Parser that sets Valid Methods)
(
func instanceof SafeJwtParserMethod and
not exists(CallNode c2, WithValidMethods wvm, NewParser m |
c2.getTarget() = m and
(
c2.getCall().getAnArgument() = wvm.getACall().asExpr() and
DataFlow::localFlow(c2.getAResult(), c.getReceiver())
)
)
or
//ParserFunc creates a new default Parser on call that accepts all methods
func instanceof SafeJwtParserFunc
) and
//Check that the Parse(function or method) does not check the Token Method field, which most likely is a check for method type
not exists(Field f |
f.hasQualifiedName(packagePathModern(), "Token", "Method") and
f.getARead().getRoot() = c.getCall().getAnArgument()
)
)
select c, "This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion"
20 changes: 14 additions & 6 deletions go/ql/src/experimental/CWE-347/JWTParsingSignature.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,21 @@
* @tags security
* external/cwe/cwe-347
*/

import go
import JWT
import DataFlow
from CallNode c, LestrratParsev1 lp
where c.getTarget() instanceof LestrratParseInsecure or c.getTarget() instanceof UnafeJWTParserMethod
or (c.getTarget() = lp and not exists(LestrratVerify lv | c.getCall().getAnArgument() = lv.getACall().asExpr()
and not(c.getCall().getArgument(0) = lv.getACall().asExpr())))
select c, "This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens"


from CallNode c, LestrratParsev1 lp
where
c.getTarget() instanceof LestrratParseInsecure
or
c.getTarget() instanceof UnafeJwtParserMethod
or
c.getTarget() = lp and
not exists(LestrratVerify lv |
c.getCall().getAnArgument() = lv.getACall().asExpr() and
not c.getCall().getArgument(0) = lv.getACall().asExpr()
)
Comment thread Fixed
select c,
"This call to Parse to accept JWT token does not check the signature, which may allow an attacker to forget tokens"
4 changes: 2 additions & 2 deletions go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
| JWTParsingAlgorithim.go:20:4:23:5 | call to Parse | This Parse Call to Verify the JWT token is vulnerable to algorithim confusion |
| JWTParsingAlgorithim.go:38:22:41:5 | call to Parse | This Parse Call to Verify the JWT token is vulnerable to algorithim confusion |
| JWTParsingAlgorithm.go:20:4:23:5 | call to Parse | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion |
| JWTParsingAlgorithm.go:38:22:41:5 | call to Parse | This Parse Call to Verify the JWT token may be vulnerable to algorithim confusion |
2 changes: 1 addition & 1 deletion go/ql/test/experimental/CWE-347/JWTParsingAlgorithm.qlref
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
experimental/CWE-347/JWTParsingAlgorithim.ql
experimental/CWE-347/JWTParsingAlgorithm.ql