False Positive: BrokenCryptoAlgorithm.ql reports nearby string literals even when the cipher call itself stays on AES.
- Ngôn ngữ chính
- CodeQL
- Star
- 10.1k
- Fork
- 2.1k
- Merge trung bình
- 2 ngày 15 giờ
- Pull request đã merge (30 ngày)
- 141
Mô tả
Version
codeql 2.24.3
## Checker
- Checker id: `Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql`
- Checker description: This checker detects insecure cryptographic algorithms (e.g., DES, RC4, ECB mode) being used as the algorithm specification in cryptographic operations.
## Description of the false positive
These samples all keep the actual cryptographic operation on a safe algorithm. What changes is that insecure-looking strings appear somewhere nearby, or an intermediate computation happens to mention part of an insecure transformation name.
That should not be enough to report `Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql`. The rule should care about the transformation that reaches `Cipher.getInstance(...)`, not unrelated literals in the same method.
## Affected test cases
### `NegCase1_Var5.java`
This case requests `AES/CBC/PKCS5Padding`. There is no insecure algorithm at the sink, so this should stay unreported.
```java
// A program with no string literal matching the insecure cryptographic algorithm pattern should not be flagged as insecure crypto usage.
package scensct.var.neg;
public class NegCase1_Var5 {
// Variant 5: Lexical and idiomatic - different secure algorithm from same family
public static void main(String[] args) throws Exception {
// Using another secure algorithm: AES/CBC/PKCS5Padding (still secure if used with proper IV)
// This tests that the checker only flags insecure ones, not all algorithms.
String secureAlg = "AES/CBC/PKCS5Padding";
javax.crypto.Cipher cipher = javax.crypto.Cipher.getInstance(secureAlg); // [REPORTED LINE]
System.out.println("Cipher created with: " + secureAlg);
}
}
```
### `NegCase3.java`
This sample keeps an `RC4`-looking string around, but the cipher call still uses `AES/CBC/PKCS5Padding`. Reporting this would mean the query is matching on lexical proximity rather than the actual call argument.
```java
// A string literal matching the insecure pattern and a cryptographic call exist, but the call uses a different secure algorithm string should not be flagged as insecure crypto usage.
package scensct.core.neg;
public class NegCase3 {
public static void main(String[] args) throws Exception {
String unusedLiteral = "RC4_NOT_USED"; // Changed value to avoid matching insecure pattern
String secureAlg = "AES/CBC/PKCS5Padding"; // Different secure algorithm
javax.crypto.Cipher cipher = javax.crypto.Cipher.getInstance(secureAlg); // Crypto call uses secure string // [REPORTED LINE]
System.out.println("Insecure literal unused: " + unusedLiteral);
}
}
```
### `NegCase4_Var5.java`
Here the code derives `"DES"` from a string, but that value never becomes the transformation passed to `Cipher.getInstance(...)`. The live sink still uses AES, so this is not a real broken-crypto finding.
```java
// A string literal matching the insecure pattern flows along a path but does not reach the algorithm parameter of a cryptographic operation should not be flagged as insecure crypto usage.
package scensct.var.neg;
public class NegCase4_Var5 {
public static void main(String[] args) throws Exception {
String alg = "DES/ECB/PKCS5Padding";
String extracted = null;
try {
extracted = alg.toUpperCase().substring(0, 3);
} catch (RuntimeException e) {
// ignore, won't happen
}
javax.crypto.Cipher cipher = javax.crypto.Cipher.getInstance("AES/CBC/PKCS5Padding"); // [REPORTED LINE]
System.out.println("Extracted: " + extracted);
}
}
```
## Cause analysis
The common failure mode is that the query seems to treat suspicious string constants as if they were equivalent to the transformation that reaches the crypto API. That is too loose for `Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql`.
In all three samples, the value at `Cipher.getInstance(...)` is still a safe AES transformation. The reported issue only appears if the query keeps too much context from nearby literals or partial string computations instead of following the exact argument that reaches the sink.
Hướng dẫn đóng góp
Hướng nghiên cứu
Bắt đầu với Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql và kiểm tra cách truy vấn xác định transformation đến được Cipher.getInstance(...). Tái hiện các báo cáo bằng NegCase1_Var5.java, NegCase3.java và NegCase4_Var5.java, sau đó xác minh rằng các lệnh gọi AES an toàn không còn được báo cáo, trong khi các transformation thực sự không an toàn vẫn được phát hiện.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- java
- Lĩnh vực
- security
- Loại issue
- Lỗi
- Độ khó
- 3/5
- Thời gian dự kiến
- 1-2 ngày
- Mức độ hoạt động
- Ít trao đổi
- Độ rõ ràng
- Khá rõ ràng
- Mức phù hợp với người mới
- 52/100