Rust: Use canonical paths for variants in data flow

github · Dec 2, 2024 · 0ebb933 · 0ebb933
1 parent e6e4f92
commit 0ebb933
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 25 deletions.
diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSummary.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSummary.qll
@@ -13,7 +13,7 @@ private module Summaries {
     UnwrapSummary() { this = "lang:core::_::<crate::option::Option>::unwrap" }
 
     override predicate propagatesFlow(string input, string output, boolean preservesValue) {
-      input = "Argument[self].Variant[crate::std::option::Option::Some(0)]" and
+      input = "Argument[self].Variant[crate::option::Option::Some(0)]" and
       output = "ReturnValue" and
       preservesValue = true
     }

diff --git a/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll b/rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll
@@ -777,15 +777,13 @@ module RustDataFlow implements InputSig<Location> {
     exists(CrateOriginOption crate, string path |
       resolveExtendedCanonicalPath(p.getQualifier(), crate, path) and
       v = MkVariantCanonicalPath(crate, path, p.getPart().getNameRef().getText())
+      or
+      exists(string name |
+        not p.hasQualifier() and
+        resolveExtendedCanonicalPath(p, crate, path + "::" + name) and
+        v = MkVariantCanonicalPath(crate, path, name)
+      )
     )
-    or
-    // TODO: Remove once library types are extracted
-    not p.hasQualifier() and
-    v = MkVariantCanonicalPath(_, "crate::std::option::Option", p.getPart().getNameRef().getText())
-    or
-    // TODO: Remove once library types are extracted
-    not p.hasQualifier() and
-    v = MkVariantCanonicalPath(_, "crate::std::result::Result", p.getPart().getNameRef().getText())
   }
 
   /** Holds if `p` destructs an enum variant `v`. */
@@ -1002,20 +1000,22 @@ private module Cached {
   cached
   newtype TReturnKind = TNormalReturnKind()
 
+  private CrateOriginOption langCoreCrate() { result.asSome() = "lang:core" }
+
   cached
   newtype TVariantCanonicalPath =
     MkVariantCanonicalPath(CrateOriginOption crate, string path, string name) {
       variantHasExtendedCanonicalPath(_, _, crate, path, name)
       or
       // TODO: Remove once library types are extracted
-      crate.isNone() and
-      path = "crate::std::option::Option" and
-      name = "Some"
-      or
-      // TODO: Remove once library types are extracted
-      crate.isNone() and
-      path = "crate::std::result::Result" and
-      name = ["Ok", "Err"]
+      crate = langCoreCrate() and
+      (
+        path = "crate::option::Option" and
+        name = "Some"
+        or
+        path = "crate::result::Result" and
+        name = ["Ok", "Err"]
+      )
     }
 
   cached
@@ -1024,11 +1024,11 @@ private module Cached {
       pos in [0 .. v.getVariant().getFieldList().(TupleFieldList).getNumberOfFields() - 1]
       or
       // TODO: Remove once library types are extracted
-      v = MkVariantCanonicalPath(_, "crate::std::option::Option", "Some") and
+      v = MkVariantCanonicalPath(langCoreCrate(), "crate::option::Option", "Some") and
       pos = 0
       or
       // TODO: Remove once library types are extracted
-      v = MkVariantCanonicalPath(_, "crate::std::result::Result", ["Ok", "Err"]) and
+      v = MkVariantCanonicalPath(langCoreCrate(), "crate::result::Result", ["Ok", "Err"]) and
       pos = 0
     } or
     TVariantFieldContent(VariantCanonicalPath v, string field) {

diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected
@@ -284,17 +284,25 @@ localStep
 | main.rs:236:22:236:22 | 2 | main.rs:236:9:236:22 | break ''block 2 |
 | main.rs:238:5:238:5 | a | main.rs:231:38:239:1 | { ... } |
 storeStep
+| main.rs:104:27:104:36 | source(...) | Some | main.rs:104:14:104:37 | ...::Some(...) |
+| main.rs:105:27:105:27 | 2 | Some | main.rs:105:14:105:28 | ...::Some(...) |
 | main.rs:117:19:117:28 | source(...) | Some | main.rs:117:14:117:29 | Some(...) |
 | main.rs:118:19:118:19 | 2 | Some | main.rs:118:14:118:20 | Some(...) |
 | main.rs:130:19:130:28 | source(...) | Some | main.rs:130:14:130:29 | Some(...) |
 | main.rs:140:29:140:38 | source(...) | A | main.rs:140:14:140:39 | ...::A(...) |
 | main.rs:141:29:141:29 | 2 | B | main.rs:141:14:141:30 | ...::B(...) |
+| main.rs:158:16:158:25 | source(...) | A | main.rs:158:14:158:26 | A(...) |
+| main.rs:159:16:159:16 | 2 | B | main.rs:159:14:159:17 | B(...) |
 | main.rs:180:18:180:27 | source(...) | C | main.rs:179:14:181:5 | ...::C {...} |
 | main.rs:182:41:182:41 | 2 | D | main.rs:182:14:182:43 | ...::D {...} |
+| main.rs:200:18:200:27 | source(...) | C | main.rs:199:14:201:5 | C {...} |
+| main.rs:202:27:202:27 | 2 | D | main.rs:202:14:202:29 | D {...} |
 | main.rs:245:27:245:27 | 0 | Some | main.rs:245:22:245:28 | Some(...) |
 readStep
-| file://:0:0:0:0 | [summary param] self in lang:core::_::<crate::option::Option>::unwrap | Some | file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::std::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::unwrap |
+| file://:0:0:0:0 | [summary param] self in lang:core::_::<crate::option::Option>::unwrap | Some | file://:0:0:0:0 | [summary] read: Argument[self].Variant[crate::option::Option::Some(0)] in lang:core::_::<crate::option::Option>::unwrap |
 | main.rs:33:9:33:15 | TupleStructPat | Some | main.rs:33:14:33:14 | _ |
+| main.rs:107:9:107:23 | TupleStructPat | Some | main.rs:107:22:107:22 | n |
+| main.rs:111:9:111:23 | TupleStructPat | Some | main.rs:111:22:111:22 | n |
 | main.rs:120:9:120:15 | TupleStructPat | Some | main.rs:120:14:120:14 | n |
 | main.rs:124:9:124:15 | TupleStructPat | Some | main.rs:124:14:124:14 | n |
 | main.rs:143:9:143:25 | TupleStructPat | A | main.rs:143:24:143:24 | n |
@@ -303,9 +311,21 @@ readStep
 | main.rs:147:30:147:46 | TupleStructPat | B | main.rs:147:45:147:45 | n |
 | main.rs:150:9:150:25 | TupleStructPat | A | main.rs:150:24:150:24 | n |
 | main.rs:151:9:151:25 | TupleStructPat | B | main.rs:151:24:151:24 | n |
+| main.rs:161:9:161:12 | TupleStructPat | A | main.rs:161:11:161:11 | n |
+| main.rs:162:9:162:12 | TupleStructPat | B | main.rs:162:11:162:11 | n |
+| main.rs:165:10:165:13 | TupleStructPat | A | main.rs:165:12:165:12 | n |
+| main.rs:165:17:165:20 | TupleStructPat | B | main.rs:165:19:165:19 | n |
+| main.rs:168:9:168:12 | TupleStructPat | A | main.rs:168:11:168:11 | n |
+| main.rs:169:9:169:12 | TupleStructPat | B | main.rs:169:11:169:11 | n |
 | main.rs:184:9:184:38 | ...::C {...} | C | main.rs:184:36:184:36 | n |
 | main.rs:185:9:185:38 | ...::D {...} | D | main.rs:185:36:185:36 | n |
 | main.rs:188:10:188:39 | ...::C {...} | C | main.rs:188:37:188:37 | n |
 | main.rs:188:43:188:72 | ...::D {...} | D | main.rs:188:70:188:70 | n |
 | main.rs:191:9:191:38 | ...::C {...} | C | main.rs:191:36:191:36 | n |
 | main.rs:192:9:192:38 | ...::D {...} | D | main.rs:192:36:192:36 | n |
+| main.rs:204:9:204:24 | C {...} | C | main.rs:204:22:204:22 | n |
+| main.rs:205:9:205:24 | D {...} | D | main.rs:205:22:205:22 | n |
+| main.rs:208:10:208:25 | C {...} | C | main.rs:208:23:208:23 | n |
+| main.rs:208:29:208:44 | D {...} | D | main.rs:208:42:208:42 | n |
+| main.rs:211:9:211:24 | C {...} | C | main.rs:211:22:211:22 | n |
+| main.rs:212:9:212:24 | D {...} | D | main.rs:212:22:212:22 | n |
diff --git a/rust/ql/test/library-tests/dataflow/local/inline-flow.expected b/rust/ql/test/library-tests/dataflow/local/inline-flow.expected
@@ -5,6 +5,10 @@ edges
 | main.rs:31:13:31:21 | source(...) | main.rs:36:10:36:10 | b | provenance |  |
 | main.rs:45:15:45:23 | source(...) | main.rs:47:10:47:10 | b | provenance |  |
 | main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | provenance |  |
+| main.rs:104:14:104:37 | ...::Some(...) [Some] | main.rs:107:9:107:23 | TupleStructPat [Some] | provenance |  |
+| main.rs:104:27:104:36 | source(...) | main.rs:104:14:104:37 | ...::Some(...) [Some] | provenance |  |
+| main.rs:107:9:107:23 | TupleStructPat [Some] | main.rs:107:22:107:22 | n | provenance |  |
+| main.rs:107:22:107:22 | n | main.rs:107:33:107:33 | n | provenance |  |
 | main.rs:117:14:117:29 | Some(...) [Some] | main.rs:120:9:120:15 | TupleStructPat [Some] | provenance |  |
 | main.rs:117:19:117:28 | source(...) | main.rs:117:14:117:29 | Some(...) [Some] | provenance |  |
 | main.rs:120:9:120:15 | TupleStructPat [Some] | main.rs:120:14:120:14 | n | provenance |  |
@@ -19,13 +23,27 @@ edges
 | main.rs:143:24:143:24 | n | main.rs:143:35:143:35 | n | provenance |  |
 | main.rs:147:10:147:26 | TupleStructPat [A] | main.rs:147:25:147:25 | n | provenance |  |
 | main.rs:147:25:147:25 | n | main.rs:147:57:147:57 | n | provenance |  |
+| main.rs:158:14:158:26 | A(...) [A] | main.rs:161:9:161:12 | TupleStructPat [A] | provenance |  |
+| main.rs:158:14:158:26 | A(...) [A] | main.rs:165:10:165:13 | TupleStructPat [A] | provenance |  |
+| main.rs:158:16:158:25 | source(...) | main.rs:158:14:158:26 | A(...) [A] | provenance |  |
+| main.rs:161:9:161:12 | TupleStructPat [A] | main.rs:161:11:161:11 | n | provenance |  |
+| main.rs:161:11:161:11 | n | main.rs:161:22:161:22 | n | provenance |  |
+| main.rs:165:10:165:13 | TupleStructPat [A] | main.rs:165:12:165:12 | n | provenance |  |
+| main.rs:165:12:165:12 | n | main.rs:165:31:165:31 | n | provenance |  |
 | main.rs:179:14:181:5 | ...::C {...} [C] | main.rs:184:9:184:38 | ...::C {...} [C] | provenance |  |
 | main.rs:179:14:181:5 | ...::C {...} [C] | main.rs:188:10:188:39 | ...::C {...} [C] | provenance |  |
 | main.rs:180:18:180:27 | source(...) | main.rs:179:14:181:5 | ...::C {...} [C] | provenance |  |
 | main.rs:184:9:184:38 | ...::C {...} [C] | main.rs:184:36:184:36 | n | provenance |  |
 | main.rs:184:36:184:36 | n | main.rs:184:48:184:48 | n | provenance |  |
 | main.rs:188:10:188:39 | ...::C {...} [C] | main.rs:188:37:188:37 | n | provenance |  |
 | main.rs:188:37:188:37 | n | main.rs:188:83:188:83 | n | provenance |  |
+| main.rs:199:14:201:5 | C {...} [C] | main.rs:204:9:204:24 | C {...} [C] | provenance |  |
+| main.rs:199:14:201:5 | C {...} [C] | main.rs:208:10:208:25 | C {...} [C] | provenance |  |
+| main.rs:200:18:200:27 | source(...) | main.rs:199:14:201:5 | C {...} [C] | provenance |  |
+| main.rs:204:9:204:24 | C {...} [C] | main.rs:204:22:204:22 | n | provenance |  |
+| main.rs:204:22:204:22 | n | main.rs:204:34:204:34 | n | provenance |  |
+| main.rs:208:10:208:25 | C {...} [C] | main.rs:208:23:208:23 | n | provenance |  |
+| main.rs:208:23:208:23 | n | main.rs:208:55:208:55 | n | provenance |  |
 nodes
 | main.rs:15:10:15:18 | source(...) | semmle.label | source(...) |
 | main.rs:19:13:19:21 | source(...) | semmle.label | source(...) |
@@ -38,6 +56,11 @@ nodes
 | main.rs:47:10:47:10 | b | semmle.label | b |
 | main.rs:53:9:53:17 | source(...) | semmle.label | source(...) |
 | main.rs:54:10:54:10 | i | semmle.label | i |
+| main.rs:104:14:104:37 | ...::Some(...) [Some] | semmle.label | ...::Some(...) [Some] |
+| main.rs:104:27:104:36 | source(...) | semmle.label | source(...) |
+| main.rs:107:9:107:23 | TupleStructPat [Some] | semmle.label | TupleStructPat [Some] |
+| main.rs:107:22:107:22 | n | semmle.label | n |
+| main.rs:107:33:107:33 | n | semmle.label | n |
 | main.rs:117:14:117:29 | Some(...) [Some] | semmle.label | Some(...) [Some] |
 | main.rs:117:19:117:28 | source(...) | semmle.label | source(...) |
 | main.rs:120:9:120:15 | TupleStructPat [Some] | semmle.label | TupleStructPat [Some] |
@@ -55,6 +78,14 @@ nodes
 | main.rs:147:10:147:26 | TupleStructPat [A] | semmle.label | TupleStructPat [A] |
 | main.rs:147:25:147:25 | n | semmle.label | n |
 | main.rs:147:57:147:57 | n | semmle.label | n |
+| main.rs:158:14:158:26 | A(...) [A] | semmle.label | A(...) [A] |
+| main.rs:158:16:158:25 | source(...) | semmle.label | source(...) |
+| main.rs:161:9:161:12 | TupleStructPat [A] | semmle.label | TupleStructPat [A] |
+| main.rs:161:11:161:11 | n | semmle.label | n |
+| main.rs:161:22:161:22 | n | semmle.label | n |
+| main.rs:165:10:165:13 | TupleStructPat [A] | semmle.label | TupleStructPat [A] |
+| main.rs:165:12:165:12 | n | semmle.label | n |
+| main.rs:165:31:165:31 | n | semmle.label | n |
 | main.rs:179:14:181:5 | ...::C {...} [C] | semmle.label | ...::C {...} [C] |
 | main.rs:180:18:180:27 | source(...) | semmle.label | source(...) |
 | main.rs:184:9:184:38 | ...::C {...} [C] | semmle.label | ...::C {...} [C] |
@@ -63,6 +94,14 @@ nodes
 | main.rs:188:10:188:39 | ...::C {...} [C] | semmle.label | ...::C {...} [C] |
 | main.rs:188:37:188:37 | n | semmle.label | n |
 | main.rs:188:83:188:83 | n | semmle.label | n |
+| main.rs:199:14:201:5 | C {...} [C] | semmle.label | C {...} [C] |
+| main.rs:200:18:200:27 | source(...) | semmle.label | source(...) |
+| main.rs:204:9:204:24 | C {...} [C] | semmle.label | C {...} [C] |
+| main.rs:204:22:204:22 | n | semmle.label | n |
+| main.rs:204:34:204:34 | n | semmle.label | n |
+| main.rs:208:10:208:25 | C {...} [C] | semmle.label | C {...} [C] |
+| main.rs:208:23:208:23 | n | semmle.label | n |
+| main.rs:208:55:208:55 | n | semmle.label | n |
 subpaths
 testFailures
 #select
@@ -72,9 +111,14 @@ testFailures
 | main.rs:36:10:36:10 | b | main.rs:31:13:31:21 | source(...) | main.rs:36:10:36:10 | b | $@ | main.rs:31:13:31:21 | source(...) | source(...) |
 | main.rs:47:10:47:10 | b | main.rs:45:15:45:23 | source(...) | main.rs:47:10:47:10 | b | $@ | main.rs:45:15:45:23 | source(...) | source(...) |
 | main.rs:54:10:54:10 | i | main.rs:53:9:53:17 | source(...) | main.rs:54:10:54:10 | i | $@ | main.rs:53:9:53:17 | source(...) | source(...) |
+| main.rs:107:33:107:33 | n | main.rs:104:27:104:36 | source(...) | main.rs:107:33:107:33 | n | $@ | main.rs:104:27:104:36 | source(...) | source(...) |
 | main.rs:120:25:120:25 | n | main.rs:117:19:117:28 | source(...) | main.rs:120:25:120:25 | n | $@ | main.rs:117:19:117:28 | source(...) | source(...) |
 | main.rs:131:10:131:20 | ... .unwrap(...) | main.rs:130:19:130:28 | source(...) | main.rs:131:10:131:20 | ... .unwrap(...) | $@ | main.rs:130:19:130:28 | source(...) | source(...) |
 | main.rs:143:35:143:35 | n | main.rs:140:29:140:38 | source(...) | main.rs:143:35:143:35 | n | $@ | main.rs:140:29:140:38 | source(...) | source(...) |
 | main.rs:147:57:147:57 | n | main.rs:140:29:140:38 | source(...) | main.rs:147:57:147:57 | n | $@ | main.rs:140:29:140:38 | source(...) | source(...) |
+| main.rs:161:22:161:22 | n | main.rs:158:16:158:25 | source(...) | main.rs:161:22:161:22 | n | $@ | main.rs:158:16:158:25 | source(...) | source(...) |
+| main.rs:165:31:165:31 | n | main.rs:158:16:158:25 | source(...) | main.rs:165:31:165:31 | n | $@ | main.rs:158:16:158:25 | source(...) | source(...) |
 | main.rs:184:48:184:48 | n | main.rs:180:18:180:27 | source(...) | main.rs:184:48:184:48 | n | $@ | main.rs:180:18:180:27 | source(...) | source(...) |
 | main.rs:188:83:188:83 | n | main.rs:180:18:180:27 | source(...) | main.rs:188:83:188:83 | n | $@ | main.rs:180:18:180:27 | source(...) | source(...) |
+| main.rs:204:34:204:34 | n | main.rs:200:18:200:27 | source(...) | main.rs:204:34:204:34 | n | $@ | main.rs:200:18:200:27 | source(...) | source(...) |
+| main.rs:208:55:208:55 | n | main.rs:200:18:200:27 | source(...) | main.rs:208:55:208:55 | n | $@ | main.rs:200:18:200:27 | source(...) | source(...) |
diff --git a/rust/ql/test/library-tests/dataflow/local/main.rs b/rust/ql/test/library-tests/dataflow/local/main.rs
@@ -104,7 +104,7 @@ fn option_pattern_match_qualified() {
     let s1 = Option::Some(source(13));
     let s2 = Option::Some(2);
     match s1 {
-        Option::Some(n) => sink(n), // $ MISSING: hasValueFlow=13
+        Option::Some(n) => sink(n), // $ hasValueFlow=13
         Option::None => sink(0),
     }
     match s2 {
@@ -158,11 +158,11 @@ fn custom_tuple_enum_pattern_match_unqualified() {
     let s1 = A(source(16));
     let s2 = B(2);
     match s1 {
-        A(n) => sink(n), // $ MISSING: hasValueFlow=16
+        A(n) => sink(n), // $ hasValueFlow=16
         B(n) => sink(n),
     }
     match s1 {
-        (A(n) | B(n)) => sink(n), // $ MISSING: hasValueFlow=16
+        (A(n) | B(n)) => sink(n), // $ hasValueFlow=16
     }
     match s2 {
         A(n) => sink(n),
@@ -201,11 +201,11 @@ fn custom_record_enum_pattern_match_unqualified() {
     };
     let s2 = D { field_d: 2 };
     match s1 {
-        C { field_c: n } => sink(n), // $ MISSING: hasValueFlow=18
+        C { field_c: n } => sink(n), // $ hasValueFlow=18
         D { field_d: n } => sink(n),
     }
     match s1 {
-        (C { field_c: n } | D { field_d: n }) => sink(n), // $ MISSING: hasValueFlow=18
+        (C { field_c: n } | D { field_d: n }) => sink(n), // $ hasValueFlow=18
     }
     match s2 {
         C { field_c: n } => sink(n),