Update shared/controlflow/codeql/controlflow/ControlFlowGraph.qll

Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>

rust/ql/test/query-tests/security/CWE-327/WeakSensitiveDataHashing/CryptographicOperations.expected

@@ -0,0 +1,12 @@
+| test.rs:19:9:19:34 | ...::compute(...) | HashingAlgorithm MD5 WEAK inputs:1 |
+| test.rs:20:9:20:40 | ...::compute(...) | HashingAlgorithm MD5 WEAK inputs:1 |
+| test.rs:21:9:21:34 | ...::compute(...) | HashingAlgorithm MD5 WEAK inputs:1 |
+| test.rs:22:9:22:44 | ...::compute(...) | HashingAlgorithm MD5 WEAK inputs:1 |
+| test.rs:67:26:67:40 | ...::new(...) | HashingAlgorithm MD5 WEAK  |
+| test.rs:73:9:73:23 | ...::new(...) | HashingAlgorithm MD5 WEAK  |
+| test.rs:74:9:74:23 | ...::new(...) | HashingAlgorithm MD5 WEAK  |
+| test.rs:133:26:133:40 | ...::new(...) | HashingAlgorithm MD5 WEAK  |
+| test.rs:156:26:156:40 | ...::new(...) | HashingAlgorithm MD5 WEAK  |
+| test.rs:176:13:176:24 | ...::new(...) | EncryptionAlgorithm SEED   |
+| test.rs:199:22:199:32 | ...::new(...) | HashingAlgorithm SHA1 WEAK  |
+| test.rs:211:13:211:35 | ...::compute(...) | HashingAlgorithm MD5 WEAK inputs:1 |

rust/ql/test/query-tests/security/CWE-327/WeakSensitiveDataHashing/CryptographicOperations.qlref

@@ -0,0 +1,3 @@
+query: queries/summary/CryptographicOperations.ql
+postprocess:
+ - utils/test/InlineExpectationsTestQuery.ql

rust/ql/test/query-tests/security/CWE-327/WeakSensitiveDataHashing/WeakSensitiveDataHashing.expected

@@ -1,9 +1,13 @@
 #select
 | test.rs:20:9:20:24 | ...::compute | test.rs:20:26:20:39 | credit_card_no | test.rs:20:9:20:24 | ...::compute | $@ is used in a hashing algorithm (MD5) that is insecure. | test.rs:20:26:20:39 | credit_card_no | Sensitive data (private) |
 | test.rs:21:9:21:24 | ...::compute | test.rs:21:26:21:33 | password | test.rs:21:9:21:24 | ...::compute | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:21:26:21:33 | password | Sensitive data (password) |
+| test.rs:211:13:211:28 | ...::compute | test.rs:226:29:226:36 | password | test.rs:211:13:211:28 | ...::compute | $@ is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. | test.rs:226:29:226:36 | password | Sensitive data (password) |
 edges
 | test.rs:20:26:20:39 | credit_card_no | test.rs:20:9:20:24 | ...::compute | provenance | MaD:1 Sink:MaD:1 |
 | test.rs:21:26:21:33 | password | test.rs:21:9:21:24 | ...::compute | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:210:20:210:30 | ...: ... | test.rs:211:30:211:34 | value | provenance |  |
+| test.rs:211:30:211:34 | value | test.rs:211:13:211:28 | ...::compute | provenance | MaD:1 Sink:MaD:1 |
+| test.rs:226:29:226:36 | password | test.rs:210:20:210:30 | ...: ... | provenance |  |
 models
 | 1 | Sink: md5::compute; Argument[0]; hasher-input |
 nodes
@@ -11,4 +15,8 @@ nodes
 | test.rs:20:26:20:39 | credit_card_no | semmle.label | credit_card_no |
 | test.rs:21:9:21:24 | ...::compute | semmle.label | ...::compute |
 | test.rs:21:26:21:33 | password | semmle.label | password |
+| test.rs:210:20:210:30 | ...: ... | semmle.label | ...: ... |
+| test.rs:211:13:211:28 | ...::compute | semmle.label | ...::compute |
+| test.rs:211:30:211:34 | value | semmle.label | value |
+| test.rs:226:29:226:36 | password | semmle.label | password |
 subpaths

rust/ql/test/query-tests/security/CWE-327/WeakSensitiveDataHashing/test.rs

@@ -16,10 +16,10 @@ fn test_hash_algorithms(
     _ = md5::Md5::digest(encrypted_password);
 
     // MD5 (alternative / older library)
-    _ = md5_alt::compute(harmless);
+    _ = md5_alt::compute(harmless); // $ Alert[rust/summary/cryptographic-operations]
-    _ = md5_alt::compute(credit_card_no); // $ Alert[rust/weak-sensitive-data-hashing]
+    _ = md5_alt::compute(credit_card_no); // $ Alert[rust/summary/cryptographic-operations] Alert[rust/weak-sensitive-data-hashing]
-    _ = md5_alt::compute(password); // $ Alert[rust/weak-sensitive-data-hashing]
+    _ = md5_alt::compute(password); // $ Alert[rust/summary/cryptographic-operations] Alert[rust/weak-sensitive-data-hashing]
-    _ = md5_alt::compute(encrypted_password);
+    _ = md5_alt::compute(encrypted_password); // $ Alert[rust/summary/cryptographic-operations]
 
     // SHA-1
     _ = sha1::Sha1::digest(harmless);
@@ -64,14 +64,14 @@ fn test_hash_code_patterns(
     _ = md5::Md5::digest(password_vec); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
 
     // hash through a hasher object
-    let mut md5_hasher = md5::Md5::new();
+    let mut md5_hasher = md5::Md5::new(); // $ Alert[rust/summary/cryptographic-operations]
     md5_hasher.update(b"abc");
     md5_hasher.update(harmless);
     md5_hasher.update(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
     _ = md5_hasher.finalize();
 
-    _ = md5::Md5::new().chain_update(harmless).chain_update(harmless).chain_update(harmless).finalize();
+    _ = md5::Md5::new().chain_update(harmless).chain_update(harmless).chain_update(harmless).finalize(); // $ Alert[rust/summary/cryptographic-operations]
-    _ = md5::Md5::new().chain_update(harmless).chain_update(password).chain_update(harmless).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+    _ = md5::Md5::new().chain_update(harmless).chain_update(password).chain_update(harmless).finalize(); // $ Alert[rust/summary/cryptographic-operations] MISSING: Alert[rust/weak-sensitive-data-hashing]
 
     _ = md5::Md5::new_with_prefix(harmless).finalize();
     _ = md5::Md5::new_with_prefix(password).finalize(); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
@@ -130,7 +130,7 @@ fn test_hash_structs() {
     let str3c = serde_urlencoded::to_string(&s3).unwrap();
 
     // hash with MD5
-    let mut md5_hasher = md5::Md5::new();
+    let mut md5_hasher = md5::Md5::new(); // $ Alert[rust/summary/cryptographic-operations]
     md5_hasher.update(s1.data);
     md5_hasher.update(s2.credit_card_no); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
     md5_hasher.update(s3.password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
@@ -153,8 +153,75 @@ fn test_hash_file(
     let mut harmless_file = std::fs::File::open(harmless_filename).unwrap();
     let mut password_file = std::fs::File::open(password_filename).unwrap();
 
-    let mut md5_hasher = md5::Md5::new();
+    let mut md5_hasher = md5::Md5::new(); // $ Alert[rust/summary/cryptographic-operations]
     _ = std::io::copy(&mut harmless_file, &mut md5_hasher);
     _ = std::io::copy(&mut password_file, &mut md5_hasher); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
     _ = md5_hasher.finalize();
 }
+
+// ---
+
+struct Seed {
+}
+
+impl Seed {
+    fn new(_seed_value: u64) -> Self {
+        Seed { }
+    }
+}
+
+fn test_seed() {
+    // this will be misrecognized as a use of the SEED algorithm, but SEED is strong and the input
+    // is not sensitive data, so `rust/weak-sensitive-data-hashing` should not report a result here.
+    let _ = Seed::new(0); // $ Alert[rust/summary/cryptographic-operations]
+}
+
+// ---
+
+struct Sha1 {
+}
+
+impl Sha1 {
+    const fn new() -> Self {
+        Sha1 { }
+    }
+
+    const fn update(&mut self, _data: &[u8]) {
+        // ...
+    }
+
+    const fn finalize(self) -> [u8; 20] {
+        [0; 20]
+    }
+}
+
+fn sha1_test(password: &[u8]) {
+    let mut hasher = Sha1::new(); // $ Alert[rust/summary/cryptographic-operations]
+    hasher.update(password); // $ MISSING: Alert[rust/weak-sensitive-data-hashing]
+    _ = hasher.finalize();
+}
+
+// ---
+
+struct HashCollection {
+}
+
+impl HashCollection {
+    pub fn add_sig(value: &str) -> Self {
+        _ = md5_alt::compute(value); // $ Alert[rust/summary/cryptographic-operations] Alert[rust/weak-sensitive-data-hashing]
+
+        // ...
+
+        HashCollection { }
+    }
+}
+
+fn test_hash_collection() {
+    // this indirectly performs MD5 hashing, but the data is not sensitive
+    let id: &str = "my_id_1234567890";
+    HashCollection::add_sig(id);
+
+    // this indirectly performs MD5 hashing, and the data is sensitive; the result is reported here
+    let password: &str = "password123";
+    HashCollection::add_sig(password); // $ Source
+}

shared/controlflow/codeql/controlflow/ControlFlowGraph.qll

@@ -224,6 +224,13 @@ signature module AstSig<LocationSig Location> {
    */
   default AstNode getTryElse(TryStmt try) { none() }
 
+  /**
+   * Gets the `else` block of loop statement `loop`, if any.
+   *
+   * Only some languages (e.g. Python) support `for-else` constructs.
+   */
+  default AstNode getLoopElse(LoopStmt loop) { none() }
+
   /** A catch clause in a try statement. */
   class CatchClause extends AstNode {
     /** Gets the variable declared by this catch clause. */
@@ -1578,19 +1585,32 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
           n2.isBefore(loopstmt.getBody())
           or
           n1.isAfterValue(cond, any(BooleanSuccessor b | b.getValue() = while.booleanNot())) and
-          n2.isAfter(loopstmt)
+          (
+            n2.isBefore(getLoopElse(loopstmt))
+            or
+            not exists(getLoopElse(loopstmt)) and n2.isAfter(loopstmt)
+          )
           or
           n1.isAfter(loopstmt.getBody()) and
           n2.isAdditional(loopstmt, loopHeaderTag())
         )
         or
+        exists(LoopStmt loopstmt |
+          n1.isAfter(getLoopElse(loopstmt)) and
+          n2.isAfter(loopstmt)
+        )
+        or
         exists(ForeachStmt foreachstmt |
           n1.isBefore(foreachstmt) and
           n2.isBefore(foreachstmt.getCollection())
           or
           n1.isAfterValue(foreachstmt.getCollection(),
             any(EmptinessSuccessor t | t.getValue() = true)) and
-          n2.isAfter(foreachstmt)
+          (
+            n2.isBefore(getLoopElse(foreachstmt))
+            or
+            not exists(getLoopElse(foreachstmt)) and n2.isAfter(foreachstmt)
+          )
           or
           n1.isAfterValue(foreachstmt.getCollection(),
             any(EmptinessSuccessor t | t.getValue() = false)) and
@@ -1603,7 +1623,11 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
           n2.isAdditional(foreachstmt, loopHeaderTag())
           or
           n1.isAdditional(foreachstmt, loopHeaderTag()) and
-          n2.isAfter(foreachstmt)
+          (
+            n2.isBefore(getLoopElse(foreachstmt))
+            or
+            not exists(getLoopElse(foreachstmt)) and n2.isAfter(foreachstmt)
+          )
           or
           n1.isAdditional(foreachstmt, loopHeaderTag()) and
           n2.isBefore(foreachstmt.getVariable())