mirror of
https://github.com/github/codeql.git
synced 2026-03-31 12:48:17 +02:00
Merge branch 'main' of https://github.com/github/codeql into post-release-prep/codeql-cli-2.25.1
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The `java/potentially-weak-cryptographic-algorithm` query no longer flags Elliptic Curve algorithms (`EC`, `ECDSA`, `ECDH`, `EdDSA`, `Ed25519`, `Ed448`, `XDH`, `X25519`, `X448`), HMAC-based algorithms (`HMACSHA1`, `HMACSHA256`, `HMACSHA384`, `HMACSHA512`), or PBKDF2 key derivation as potentially insecure. These are modern, secure algorithms recommended by NIST and other standards bodies. This will reduce the number of false positives for this query.
|
||||
* The first argument of the method `getInstance` of `java.security.Signature` is now modeled as a sink for `java/potentially-weak-cryptographic-algorithm`, `java/weak-cryptographic-algorithm` and `java/rsa-without-oaep`. This will increase the number of alerts for these queries.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The `java/tainted-arithmetic` query no longer flags arithmetic expressions that are used directly as an operand of a comparison in `if`-condition bounds-checking patterns. For example, `if (off + len > array.length)` is now recognized as a bounds check rather than a potentially vulnerable computation, reducing false positives.
|
||||
@@ -132,7 +132,21 @@ private predicate inBitwiseAnd(Expr exp) {
|
||||
/** Holds if overflow/underflow is irrelevant for this expression. */
|
||||
predicate overflowIrrelevant(Expr exp) {
|
||||
inBitwiseAnd(exp) or
|
||||
exp.getEnclosingCallable() instanceof HashCodeMethod
|
||||
exp.getEnclosingCallable() instanceof HashCodeMethod or
|
||||
arithmeticUsedInBoundsCheck(exp)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `exp` is an arithmetic expression used directly as an operand of a
|
||||
* comparison in an `if`-condition, indicating it is part of a bounds check
|
||||
* rather than a vulnerable computation. For example, in
|
||||
* `if (off + len > array.length)`, the addition is the bounds check itself.
|
||||
*/
|
||||
private predicate arithmeticUsedInBoundsCheck(ArithExpr exp) {
|
||||
exists(ComparisonExpr comp |
|
||||
comp.getAnOperand() = exp and
|
||||
comp.getEnclosingStmt() instanceof IfStmt
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -259,7 +259,13 @@ string getASecureAlgorithmName() {
|
||||
result =
|
||||
[
|
||||
"RSA", "SHA-?(256|384|512)", "CCM", "GCM", "AES(?)",
|
||||
"Blowfish", "ECIES", "SHA3-(256|384|512)"
|
||||
"Blowfish", "ECIES", "SHA3-(256|384|512)",
|
||||
// Elliptic Curve algorithms: EC (key generation), ECDSA (signatures), ECDH (key agreement),
|
||||
// EdDSA/Ed25519/Ed448 (Edwards-curve signatures), XDH/X25519/X448 (key agreement).
|
||||
// These are modern, secure algorithms recommended by NIST and other standards bodies.
|
||||
"EC", "ECDSA", "ECDH", "EdDSA", "Ed25519", "Ed448", "XDH", "X25519", "X448",
|
||||
// HMAC-based algorithms and key derivation functions.
|
||||
"HMACSHA(1|256|384|512)", "HmacSHA(1|256|384|512)", "PBKDF2"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -366,9 +372,13 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec {
|
||||
exists(Constructor c | c.getAReference() = this |
|
||||
c.getDeclaringType().hasQualifiedName("java.security", "Signature")
|
||||
)
|
||||
or
|
||||
exists(Method m | m.getAReference() = this |
|
||||
m.hasQualifiedName("java.security", "Signature", "getInstance")
|
||||
)
|
||||
}
|
||||
|
||||
override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
|
||||
override Expr getAlgoSpec() { result = this.(Call).getArgument(0) }
|
||||
}
|
||||
|
||||
/** A call to the `getInstance` method declared in `java.security.KeyPairGenerator`. */
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* @description Exposing a Java object in a WebView with a JavaScript interface can lead to malicious JavaScript controlling the application.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 6.1
|
||||
* @security-severity 7.8
|
||||
* @precision medium
|
||||
* @tags security
|
||||
* external/cwe/cwe-079
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* @kind problem
|
||||
* @id java/android/websettings-javascript-enabled
|
||||
* @problem.severity warning
|
||||
* @security-severity 6.1
|
||||
* @security-severity 7.8
|
||||
* @precision medium
|
||||
* @tags security
|
||||
* external/cwe/cwe-079
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* allows for a cross-site scripting vulnerability.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @security-severity 6.1
|
||||
* @security-severity 7.8
|
||||
* @precision high
|
||||
* @id java/xss
|
||||
* @tags security
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* insertion of forged log entries by malicious users.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @security-severity 7.8
|
||||
* @security-severity 6.1
|
||||
* @precision medium
|
||||
* @id java/log-injection
|
||||
* @tags security
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
category: queryMetadata
|
||||
---
|
||||
* The `@security-severity` metadata of `java/log-injection` has been reduced from 7.8 (high) to 6.1 (medium).
|
||||
* The `@security-severity` metadata of `java/android/webview-addjavascriptinterface`, `java/android/websettings-javascript-enabled` and `java/xss` has been increased from 6.1 (medium) to 7.8 (high).
|
||||
@@ -4,10 +4,10 @@
|
||||
| ArithmeticTainted.java:50:17:50:24 | ... + ... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:50:17:50:20 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:71:17:71:27 | ... + ... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:71:17:71:23 | herring | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:95:37:95:46 | ... + ... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:95:37:95:40 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:127:3:127:8 | ...++ | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:127:3:127:6 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:131:3:131:8 | ++... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:131:5:131:8 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:135:3:135:8 | ...-- | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:135:3:135:6 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:139:3:139:8 | --... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:139:5:139:8 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:129:3:129:8 | ...++ | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:129:3:129:6 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:133:3:133:8 | ++... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:133:5:133:8 | data | This arithmetic expression depends on a $@, potentially causing an overflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:137:3:137:8 | ...-- | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:137:3:137:6 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
| ArithmeticTainted.java:141:3:141:8 | --... | ArithmeticTainted.java:17:46:17:54 | System.in : InputStream | ArithmeticTainted.java:141:5:141:8 | data | This arithmetic expression depends on a $@, potentially causing an underflow. | ArithmeticTainted.java:17:46:17:54 | System.in | user-provided value |
|
||||
edges
|
||||
| ArithmeticTainted.java:17:24:17:64 | new InputStreamReader(...) : InputStreamReader | ArithmeticTainted.java:18:40:18:56 | readerInputStream : InputStreamReader | provenance | |
|
||||
| ArithmeticTainted.java:17:24:17:64 | new InputStreamReader(...) : InputStreamReader | ArithmeticTainted.java:18:40:18:56 | readerInputStream : InputStreamReader | provenance | |
|
||||
@@ -38,14 +38,14 @@ edges
|
||||
| ArithmeticTainted.java:66:18:66:24 | tainted : Holder [dat] : Number | ArithmeticTainted.java:66:18:66:34 | getData(...) : Number | provenance | |
|
||||
| ArithmeticTainted.java:66:18:66:24 | tainted : Holder [dat] : Number | Holder.java:16:13:16:19 | parameter this : Holder [dat] : Number | provenance | |
|
||||
| ArithmeticTainted.java:66:18:66:34 | getData(...) : Number | ArithmeticTainted.java:71:17:71:23 | herring | provenance | |
|
||||
| ArithmeticTainted.java:118:9:118:12 | data : Number | ArithmeticTainted.java:125:26:125:33 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:119:10:119:13 | data : Number | ArithmeticTainted.java:129:27:129:34 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:120:10:120:13 | data : Number | ArithmeticTainted.java:133:27:133:34 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:121:10:121:13 | data : Number | ArithmeticTainted.java:137:27:137:34 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:125:26:125:33 | data : Number | ArithmeticTainted.java:127:3:127:6 | data | provenance | |
|
||||
| ArithmeticTainted.java:129:27:129:34 | data : Number | ArithmeticTainted.java:131:5:131:8 | data | provenance | |
|
||||
| ArithmeticTainted.java:133:27:133:34 | data : Number | ArithmeticTainted.java:135:3:135:6 | data | provenance | |
|
||||
| ArithmeticTainted.java:137:27:137:34 | data : Number | ArithmeticTainted.java:139:5:139:8 | data | provenance | |
|
||||
| ArithmeticTainted.java:118:9:118:12 | data : Number | ArithmeticTainted.java:127:26:127:33 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:119:10:119:13 | data : Number | ArithmeticTainted.java:131:27:131:34 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:120:10:120:13 | data : Number | ArithmeticTainted.java:135:27:135:34 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:121:10:121:13 | data : Number | ArithmeticTainted.java:139:27:139:34 | data : Number | provenance | |
|
||||
| ArithmeticTainted.java:127:26:127:33 | data : Number | ArithmeticTainted.java:129:3:129:6 | data | provenance | |
|
||||
| ArithmeticTainted.java:131:27:131:34 | data : Number | ArithmeticTainted.java:133:5:133:8 | data | provenance | |
|
||||
| ArithmeticTainted.java:135:27:135:34 | data : Number | ArithmeticTainted.java:137:3:137:6 | data | provenance | |
|
||||
| ArithmeticTainted.java:139:27:139:34 | data : Number | ArithmeticTainted.java:141:5:141:8 | data | provenance | |
|
||||
| Holder.java:12:22:12:26 | d : Number | Holder.java:13:9:13:9 | d : Number | provenance | |
|
||||
| Holder.java:13:3:13:5 | this <.field> [post update] : Holder [dat] : Number | Holder.java:12:14:12:20 | parameter this [Return] : Holder [dat] : Number | provenance | |
|
||||
| Holder.java:13:9:13:9 | d : Number | Holder.java:13:3:13:5 | this <.field> [post update] : Holder [dat] : Number | provenance | |
|
||||
@@ -86,14 +86,14 @@ nodes
|
||||
| ArithmeticTainted.java:119:10:119:13 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:120:10:120:13 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:121:10:121:13 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:125:26:125:33 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:127:3:127:6 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:129:27:129:34 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:131:5:131:8 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:133:27:133:34 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:135:3:135:6 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:137:27:137:34 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:139:5:139:8 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:127:26:127:33 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:129:3:129:6 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:131:27:131:34 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:133:5:133:8 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:135:27:135:34 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:137:3:137:6 | data | semmle.label | data |
|
||||
| ArithmeticTainted.java:139:27:139:34 | data : Number | semmle.label | data : Number |
|
||||
| ArithmeticTainted.java:141:5:141:8 | data | semmle.label | data |
|
||||
| Holder.java:12:14:12:20 | parameter this [Return] : Holder [dat] : Number | semmle.label | parameter this [Return] : Holder [dat] : Number |
|
||||
| Holder.java:12:22:12:26 | d : Number | semmle.label | d : Number |
|
||||
| Holder.java:13:3:13:5 | this <.field> [post update] : Holder [dat] : Number | semmle.label | this <.field> [post update] : Holder [dat] : Number |
|
||||
|
||||
@@ -119,6 +119,8 @@ public class ArithmeticTainted {
|
||||
test2(data);
|
||||
test3(data);
|
||||
test4(data);
|
||||
boundsCheckGood(null, data, 5);
|
||||
boundsCheckGood2(null, data, 5);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -138,4 +140,18 @@ public class ArithmeticTainted {
|
||||
// BAD: may underflow if input data is very small
|
||||
--data;
|
||||
}
|
||||
|
||||
public static void boundsCheckGood(byte[] bs, int off, int len) {
|
||||
// GOOD: arithmetic used directly in a bounds check, not as a computation
|
||||
if (off + len > bs.length) {
|
||||
throw new IndexOutOfBoundsException();
|
||||
}
|
||||
}
|
||||
|
||||
public static void boundsCheckGood2(int[] arr, int offset, int count) {
|
||||
// GOOD: subtraction used directly in a bounds check
|
||||
if (offset - count < 0) {
|
||||
throw new IndexOutOfBoundsException();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,6 +46,48 @@ class Test {
|
||||
cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec);
|
||||
|
||||
byte[] encrypted = cipher.doFinal(input.getBytes("UTF-8"));
|
||||
|
||||
KeyPairGenerator keyPairGenerator;
|
||||
|
||||
// GOOD: EC is a secure algorithm for key pair generation
|
||||
keyPairGenerator = KeyPairGenerator.getInstance("EC");
|
||||
|
||||
// GOOD: ECDSA is a secure signature algorithm
|
||||
Signature ecdsaSig = Signature.getInstance("ECDSA");
|
||||
|
||||
// GOOD: ECDH is a secure algorithm for key agreement
|
||||
KeyAgreement ecdhKa = KeyAgreement.getInstance("ECDH");
|
||||
|
||||
// GOOD: EdDSA is a secure algorithm (Edwards-curve Digital Signature Algorithm)
|
||||
keyPairGenerator = KeyPairGenerator.getInstance("EdDSA");
|
||||
|
||||
// GOOD: Ed25519 is a secure algorithm for key pair generation
|
||||
keyPairGenerator = KeyPairGenerator.getInstance("Ed25519");
|
||||
|
||||
// GOOD: Ed448 is a secure algorithm for key pair generation
|
||||
keyPairGenerator = KeyPairGenerator.getInstance("Ed448");
|
||||
|
||||
// GOOD: XDH is a secure algorithm for key agreement
|
||||
KeyAgreement xdhKa = KeyAgreement.getInstance("XDH");
|
||||
|
||||
// GOOD: X25519 is a secure algorithm for key agreement
|
||||
KeyAgreement x25519Ka = KeyAgreement.getInstance("X25519");
|
||||
|
||||
// GOOD: X448 is a secure algorithm for key agreement
|
||||
KeyAgreement x448Ka = KeyAgreement.getInstance("X448");
|
||||
|
||||
// GOOD: SHA256withECDSA is a secure signature algorithm
|
||||
Signature sha256Ecdsa = Signature.getInstance("SHA256withECDSA");
|
||||
|
||||
// GOOD: HMAC-based SecretKeySpec should not be flagged
|
||||
new SecretKeySpec(null, "HMACSHA1");
|
||||
new SecretKeySpec(null, "HMACSHA256");
|
||||
new SecretKeySpec(null, "HMACSHA384");
|
||||
new SecretKeySpec(null, "SHA384withECDSA");
|
||||
|
||||
// GOOD: PBKDF2 key derivation is a secure algorithm
|
||||
SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
|
||||
|
||||
} catch (Exception e) {
|
||||
// fail
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user