Include suggested changes from review.

This commit is contained in:
Porcupiney Hairs
2022-05-31 17:17:54 +05:30
parent bd1ddc177e
commit e0f74a51ac
4 changed files with 52 additions and 38 deletions

View File

@@ -18,7 +18,7 @@
<recommendation>
<p>
Generating a crytograhically secure secret key during application initialization and using this generated key for future JWT signing requests can prevent this vulnerability.
Generating a cryptograhically secure secret key during application initialization and using this generated key for future JWT signing requests can prevent this vulnerability.
</p>
</recommendation>

View File

@@ -65,26 +65,20 @@ module HardcodedKeys {
"github.com/form3tech-oss/jwt-go", "github.com/ory/fosite/token/jwt"
]
|
(
exists(DataFlow::MethodCallNode m |
// Models the `SignedString` method
// `func (t *Token) SignedString(key interface{}) (string, error)`
m.getTarget().hasQualifiedName(pkg, "Token", "SignedString")
|
this = m.getArgument(0)
)
exists(DataFlow::MethodCallNode m |
// Models the `SignedString` method
// `func (t *Token) SignedString(key interface{}) (string, error)`
m.getTarget().hasQualifiedName(pkg, "Token", "SignedString") and
this = m.getArgument(0)
or
exists(DataFlow::MethodCallNode m |
// Model the `Sign` method of the `SigningMethod` interface
// type SigningMethod interface {
// Verify(signingString, signature string, key interface{}) error
// Sign(signingString string, key interface{}) (string, error)
// Alg() string
// }
m.getTarget().hasQualifiedName(pkg, "SigningMethod", "Sign")
|
this = m.getArgument(1)
)
// Model the `Sign` method of the `SigningMethod` interface
// type SigningMethod interface {
// Verify(signingString, signature string, key interface{}) error
// Sign(signingString string, key interface{}) (string, error)
// Alg() string
// }
m.getTarget().hasQualifiedName(pkg, "SigningMethod", "Sign") and
this = m.getArgument(1)
)
)
}
@@ -240,12 +234,21 @@ module HardcodedKeys {
// }
exists(DataFlow::CallNode randread, DataFlow::Node rand, DataFlow::ElementReadNode r |
randread.getTarget().hasQualifiedName("crypto/rand", "Read") and
TaintTracking::localTaint(randread.getArgument(0).getAPredecessor*().getASuccessor*(), rand) and
TaintTracking::localTaint(any(DataFlow::PostUpdateNode pun |
pun.getPreUpdateNode() = randread.getArgument(0)
), rand) and
(
// Flow through a ModExpr if any of the operands are tainted.
// For ex, in the case shown above,
// `bytes[i] = characters[x%byte(len(characters))]`
// given x is cryptographically secure random number,
// we can assume that `bytes` is random and cryptographically secure.
exists(ModExpr e | e.getAnOperand() = rand.asExpr() |
r.reads(this, e.getGlobalValueNumber().getANode())
)
or
// This is an alternative case where the code uses `x` directly instead
// `bytes[i] = characters[x]`
r.reads(this.getAPredecessor*(), rand)
)
)

View File

@@ -14,12 +14,16 @@ edges
| main.go:80:16:80:21 | "key6" : string | main.go:80:9:80:22 | type conversion : string |
| main.go:89:10:89:23 | type conversion : string | main.go:91:66:91:69 | key2 |
| main.go:89:17:89:22 | "key7" : string | main.go:89:10:89:23 | type conversion : string |
| main.go:97:9:97:22 | type conversion : string | main.go:103:30:103:32 | key |
| main.go:97:9:97:22 | type conversion : string | main.go:102:30:102:32 | key |
| main.go:97:16:97:21 | "key8" : string | main.go:97:9:97:22 | type conversion : string |
| main.go:107:15:107:28 | type conversion : string | main.go:108:16:108:24 | sharedKey |
| main.go:107:22:107:27 | "key9" : string | main.go:107:15:107:28 | type conversion : string |
| main.go:111:23:111:37 | type conversion : string | main.go:114:16:114:30 | sharedKeyglobal |
| main.go:111:30:111:36 | "key10" : string | main.go:111:23:111:37 | type conversion : string |
| main.go:106:15:106:28 | type conversion : string | main.go:107:16:107:24 | sharedKey |
| main.go:106:22:106:27 | "key9" : string | main.go:106:15:106:28 | type conversion : string |
| main.go:110:23:110:37 | type conversion : string | main.go:113:16:113:30 | sharedKeyglobal |
| main.go:110:30:110:36 | "key10" : string | main.go:110:23:110:37 | type conversion : string |
| sanitizer.go:17:9:17:21 | type conversion : string | sanitizer.go:18:44:18:46 | key |
| sanitizer.go:17:16:17:20 | `key` : string | sanitizer.go:17:9:17:21 | type conversion : string |
| sanitizer.go:80:10:80:14 | "asd" : string | sanitizer.go:99:2:99:24 | ... := ...[0] : string |
| sanitizer.go:99:2:99:24 | ... := ...[0] : string | sanitizer.go:104:44:104:47 | key4 |
nodes
| HardcodedKeysBad.go:11:18:11:38 | type conversion : string | semmle.label | type conversion : string |
| HardcodedKeysBad.go:11:25:11:37 | "AllYourBase" : string | semmle.label | "AllYourBase" : string |
@@ -46,13 +50,19 @@ nodes
| main.go:91:66:91:69 | key2 | semmle.label | key2 |
| main.go:97:9:97:22 | type conversion : string | semmle.label | type conversion : string |
| main.go:97:16:97:21 | "key8" : string | semmle.label | "key8" : string |
| main.go:103:30:103:32 | key | semmle.label | key |
| main.go:107:15:107:28 | type conversion : string | semmle.label | type conversion : string |
| main.go:107:22:107:27 | "key9" : string | semmle.label | "key9" : string |
| main.go:108:16:108:24 | sharedKey | semmle.label | sharedKey |
| main.go:111:23:111:37 | type conversion : string | semmle.label | type conversion : string |
| main.go:111:30:111:36 | "key10" : string | semmle.label | "key10" : string |
| main.go:114:16:114:30 | sharedKeyglobal | semmle.label | sharedKeyglobal |
| main.go:102:30:102:32 | key | semmle.label | key |
| main.go:106:15:106:28 | type conversion : string | semmle.label | type conversion : string |
| main.go:106:22:106:27 | "key9" : string | semmle.label | "key9" : string |
| main.go:107:16:107:24 | sharedKey | semmle.label | sharedKey |
| main.go:110:23:110:37 | type conversion : string | semmle.label | type conversion : string |
| main.go:110:30:110:36 | "key10" : string | semmle.label | "key10" : string |
| main.go:113:16:113:30 | sharedKeyglobal | semmle.label | sharedKeyglobal |
| sanitizer.go:17:9:17:21 | type conversion : string | semmle.label | type conversion : string |
| sanitizer.go:17:16:17:20 | `key` : string | semmle.label | `key` : string |
| sanitizer.go:18:44:18:46 | key | semmle.label | key |
| sanitizer.go:80:10:80:14 | "asd" : string | semmle.label | "asd" : string |
| sanitizer.go:99:2:99:24 | ... := ...[0] : string | semmle.label | ... := ...[0] : string |
| sanitizer.go:104:44:104:47 | key4 | semmle.label | key4 |
subpaths
#select
| HardcodedKeysBad.go:19:28:19:39 | mySigningKey | HardcodedKeysBad.go:11:25:11:37 | "AllYourBase" : string | HardcodedKeysBad.go:19:28:19:39 | mySigningKey | $@ is used to sign a JWT token. | HardcodedKeysBad.go:11:25:11:37 | "AllYourBase" | Hardcoded String |
@@ -63,6 +73,8 @@ subpaths
| main.go:74:15:74:18 | key2 | main.go:69:17:69:22 | "key5" : string | main.go:74:15:74:18 | key2 | $@ is used to sign a JWT token. | main.go:69:17:69:22 | "key5" | Hardcoded String |
| main.go:84:41:84:43 | key | main.go:80:16:80:21 | "key6" : string | main.go:84:41:84:43 | key | $@ is used to sign a JWT token. | main.go:80:16:80:21 | "key6" | Hardcoded String |
| main.go:91:66:91:69 | key2 | main.go:89:17:89:22 | "key7" : string | main.go:91:66:91:69 | key2 | $@ is used to sign a JWT token. | main.go:89:17:89:22 | "key7" | Hardcoded String |
| main.go:103:30:103:32 | key | main.go:97:16:97:21 | "key8" : string | main.go:103:30:103:32 | key | $@ is used to sign a JWT token. | main.go:97:16:97:21 | "key8" | Hardcoded String |
| main.go:108:16:108:24 | sharedKey | main.go:107:22:107:27 | "key9" : string | main.go:108:16:108:24 | sharedKey | $@ is used to sign a JWT token. | main.go:107:22:107:27 | "key9" | Hardcoded String |
| main.go:114:16:114:30 | sharedKeyglobal | main.go:111:30:111:36 | "key10" : string | main.go:114:16:114:30 | sharedKeyglobal | $@ is used to sign a JWT token. | main.go:111:30:111:36 | "key10" | Hardcoded String |
| main.go:102:30:102:32 | key | main.go:97:16:97:21 | "key8" : string | main.go:102:30:102:32 | key | $@ is used to sign a JWT token. | main.go:97:16:97:21 | "key8" | Hardcoded String |
| main.go:107:16:107:24 | sharedKey | main.go:106:22:106:27 | "key9" : string | main.go:107:16:107:24 | sharedKey | $@ is used to sign a JWT token. | main.go:106:22:106:27 | "key9" | Hardcoded String |
| main.go:113:16:113:30 | sharedKeyglobal | main.go:110:30:110:36 | "key10" : string | main.go:113:16:113:30 | sharedKeyglobal | $@ is used to sign a JWT token. | main.go:110:30:110:36 | "key10" | Hardcoded String |
| sanitizer.go:18:44:18:46 | key | sanitizer.go:17:16:17:20 | `key` : string | sanitizer.go:18:44:18:46 | key | $@ is used to sign a JWT token. | sanitizer.go:17:16:17:20 | `key` | Hardcoded String |
| sanitizer.go:104:44:104:47 | key4 | sanitizer.go:80:10:80:14 | "asd" : string | sanitizer.go:104:44:104:47 | key4 | $@ is used to sign a JWT token. | sanitizer.go:80:10:80:14 | "asd" | Hardcoded String |

View File

@@ -7,7 +7,7 @@ package main
//go:generate depstubber -vendor github.com/lestrrat/go-jwx/jwk "" New
//go:generate depstubber -vendor github.com/square/go-jose/v3 Recipient NewEncrypter,NewSigner
//go:generate depstubber -vendor gopkg.in/square/go-jose.v2 Recipient NewEncrypter,NewSigner
////go:generate depstubber -vendor github.com/cristalhq/jwt/v3 Signer NewSignerHS,HS256
//go:generate depstubber -vendor github.com/cristalhq/jwt/v3 Signer NewSignerHS,HS256
import (
"time"
@@ -98,7 +98,6 @@ func go_kit() interface{} {
mapClaims = gjwt.MapClaims{"user": "go-kit"}
)
// e := func(ctx context.Context, i interface{}) (interface{}, error) { return ctx, nil }
return gokit.NewSigner(kid, key, nil, mapClaims) // BAD
}