Skip to content

Commit 993c1c4

Browse files
authored
Fix incorrect detection of fixed iv in G407 (#1509)
Signed-off-by: Cosmin Cojocar <cosmin@cojocar.ch>
1 parent 8668b74 commit 993c1c4

2 files changed

Lines changed: 119 additions & 0 deletions

File tree

analyzers/hardcoded_nonce.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,17 @@ const defaultIssueDescription = "Use of hardcoded IV/nonce for encryption"
3636
// and the index of the argument that is the nonce/IV.
3737
// Example: "crypto/cipher.NewCBCEncrypter": {2, 1} means the function accepts 2 arguments,
3838
// and the nonce arg is at index 1 (the second argument).
39+
// Note: We only track encryption functions, not decryption functions (like NewCBCDecrypter, NewCFBDecrypter, etc.)
40+
// because decryption must use the same nonce as encryption, which will naturally appear as a known/hardcoded value.
3941
var tracked = map[string][]int{
4042
"(crypto/cipher.AEAD).Seal": {4, 1},
4143
"crypto/cipher.NewCBCEncrypter": {2, 1},
4244
"crypto/cipher.NewCFBEncrypter": {2, 1},
45+
"crypto/cipher.NewCTREncrypter": {2, 1},
4346
"crypto/cipher.NewCTR": {2, 1},
4447
"crypto/cipher.NewOFB": {2, 1},
48+
"crypto/cipher.NewCFB": {2, 1},
49+
"crypto/cipher.NewCBC": {2, 1},
4550
}
4651

4752
var dynamicFuncs = map[string]bool{
@@ -146,6 +151,16 @@ func (s *analysisState) Release() {
146151
s.BaseAnalyzerState.Release()
147152
}
148153

154+
// isAEADOpenCall checks if a call is to AEAD.Open (decryption), which should not be flagged.
155+
func isAEADOpenCall(c *ssa.Call) bool {
156+
if c.Call.IsInvoke() && c.Call.Method != nil {
157+
name := c.Call.Method.FullName()
158+
// Check if this is (crypto/cipher.AEAD).Open
159+
return strings.Contains(name, "AEAD") && strings.HasSuffix(name, "Open")
160+
}
161+
return false
162+
}
163+
149164
// getInitialArgs is now unified in util.go TraverseSSA or kept here if specific.
150165
// It seems specific to tracked functions, so we keep it but can use TraverseSSA.
151166
func (s *analysisState) getInitialArgs(tracked map[string][]int) []ssaValueAndInstr {
@@ -154,6 +169,10 @@ func (s *analysisState) getInitialArgs(tracked map[string][]int) []ssaValueAndIn
154169
if c, ok := i.(*ssa.Call); ok {
155170
if c.Call.IsInvoke() {
156171
// Handle interface method calls (e.g. (crypto/cipher.AEAD).Seal)
172+
// Skip AEAD.Open (decryption) as it must use the same nonce as encryption
173+
if isAEADOpenCall(c) {
174+
return
175+
}
157176
name := c.Call.Method.FullName()
158177
if info, ok := tracked[name]; ok {
159178
if len(c.Call.Args) == info[0] {

testutils/g407_samples.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,5 +1256,105 @@ func main() {
12561256
block, _ := aes.NewCipher([]byte("12345678123456781234567812345678"))
12571257
_ = cipher.NewCTR(block, iv)
12581258
}
1259+
`}, 1, gosec.NewConfig()},
1260+
1261+
// Decryption tests - should NOT be flagged as decryption uses the same nonce as encryption
1262+
{[]string{`package main
1263+
1264+
import (
1265+
"crypto/aes"
1266+
"crypto/cipher"
1267+
)
1268+
1269+
func Decrypt(data []byte, key [32]byte) ([]byte, error) {
1270+
block, _ := aes.NewCipher(key[:32])
1271+
gcm, _ := cipher.NewGCM(block)
1272+
// Using a hardcoded nonce for DECRYPTION is safe - must match encryption nonce
1273+
nonce := []byte("ILoveMyNonce")
1274+
return gcm.Open(nil, nonce, data[gcm.NonceSize():], nil)
1275+
}
1276+
`}, 0, gosec.NewConfig()},
1277+
1278+
{[]string{`package main
1279+
1280+
import (
1281+
"crypto/aes"
1282+
"crypto/cipher"
1283+
)
1284+
1285+
func main() {
1286+
block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1})
1287+
aesGCM, _ := cipher.NewGCM(block)
1288+
1289+
// Encrypt with hardcoded nonce - SHOULD be flagged
1290+
cipherText := aesGCM.Seal(nil, []byte("ILoveMyNonce"), []byte("My secret message"), nil)
1291+
1292+
// Decrypt with same nonce - should NOT be flagged (same nonce as encryption)
1293+
cipherText, _ = aesGCM.Open(nil, []byte("ILoveMyNonce"), cipherText, nil)
1294+
}
1295+
`}, 1, gosec.NewConfig()},
1296+
1297+
{[]string{`package main
1298+
1299+
import (
1300+
"crypto/aes"
1301+
"crypto/cipher"
1302+
)
1303+
1304+
func main() {
1305+
block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1})
1306+
// NewCBCDecrypter should not be flagged - decryption must use same nonce as encryption
1307+
aesCBC := cipher.NewCBCDecrypter(block, []byte("ILoveMyNonceAlot"))
1308+
var output = make([]byte, 16)
1309+
aesCBC.CryptBlocks(output, []byte("encrypted_block!"))
1310+
}
1311+
`}, 0, gosec.NewConfig()},
1312+
1313+
{[]string{`package main
1314+
1315+
import (
1316+
"crypto/aes"
1317+
"crypto/cipher"
1318+
)
1319+
1320+
func main() {
1321+
block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1})
1322+
// NewCFBDecrypter should not be flagged - decryption must use same nonce as encryption
1323+
aesCFB := cipher.NewCFBDecrypter(block, []byte("ILoveMyNonceAlot"))
1324+
var output = make([]byte, 16)
1325+
aesCFB.XORKeyStream(output, []byte("Very Cool thing!"))
1326+
}
1327+
`}, 0, gosec.NewConfig()},
1328+
1329+
{[]string{`package main
1330+
1331+
import (
1332+
"crypto/aes"
1333+
"crypto/cipher"
1334+
)
1335+
1336+
func main() {
1337+
block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1})
1338+
// NewCBCEncrypter SHOULD be flagged - encryption should use random nonce
1339+
aesCBC := cipher.NewCBCEncrypter(block, []byte("ILoveMyNonceAlot"))
1340+
var output = make([]byte, 16)
1341+
aesCBC.CryptBlocks(output, []byte("Very Cool thing!"))
1342+
}
1343+
`}, 1, gosec.NewConfig()},
1344+
1345+
{[]string{`package main
1346+
1347+
import (
1348+
"crypto/aes"
1349+
"crypto/cipher"
1350+
)
1351+
1352+
func main() {
1353+
block, _ := aes.NewCipher([]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1})
1354+
// NewCFBEncrypter SHOULD be flagged - encryption should use random nonce
1355+
aesCFB := cipher.NewCFBEncrypter(block, []byte("ILoveMyNonceAlot"))
1356+
var output = make([]byte, 16)
1357+
aesCFB.XORKeyStream(output, []byte("Very Cool thing!"))
1358+
}
12591359
`}, 1, gosec.NewConfig()},
12601360
}

0 commit comments

Comments
 (0)