Shared: Prefer source/sink models with manual provenance over generated

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll

@@ -60,7 +60,7 @@ private module DispatchImpl {
       not (
         // Only use summarized callables with generated summaries in case
         // the static call target is not in the source code.
-        // Note that if applyGeneratedModel holds it implies that there doesn't
+        // Note that if `applyGeneratedModel` holds it implies that there doesn't
         // exist a manual model.
         exists(Callable staticTarget | staticTarget = call.getCallee().getSourceDeclaration() |
           staticTarget.fromSource() and not staticTarget.isStub()

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -445,7 +445,15 @@ module RustDataFlow implements InputSig<Location> {
       or
       exists(SummarizedCallable sc, Function staticTarget |
         staticTarget = getStaticTargetExt(c) and
-        sc = result.asSummarizedCallable()
+        sc = result.asSummarizedCallable() and
+        // Only use summarized callables with generated summaries in case
+        // the static call target is not in the source code.
+        // Note that if `applyGeneratedModel` holds it implies that there doesn't
+        // exist a manual model.
+        not (
+          staticTarget.fromSource() and
+          sc.applyGeneratedModel()
+        )
       |
         sc = staticTarget
         or

rust/ql/test/library-tests/dataflow/sources/file/TaintSources.expected

@@ -1,9 +1,6 @@
 | test.rs:12:31:12:43 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
-| test.rs:12:31:12:43 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
-| test.rs:17:31:17:38 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:17:31:17:38 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:22:22:22:39 | ...::read_to_string | Flow source 'FileSource' of type file (DEFAULT). |
-| test.rs:22:22:22:39 | ...::read_to_string | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:26:18:26:29 | ...::read_dir | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:29:22:29:25 | path | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:43:27:43:35 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
@@ -15,8 +12,6 @@
 | test.rs:79:31:79:45 | ...::read | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:84:22:84:46 | ...::read_to_string | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:90:26:90:29 | path | Flow source 'FileSource' of type file (DEFAULT). |
-| test.rs:90:26:90:29 | path | Flow source 'FileSource' of type file (DEFAULT). |
-| test.rs:91:31:91:39 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:91:31:91:39 | file_name | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:97:22:97:41 | ...::read_link | Flow source 'FileSource' of type file (DEFAULT). |
 | test.rs:107:20:107:38 | ...::open | Flow source 'FileSource' of type file (DEFAULT). |

rust/ql/test/query-tests/security/CWE-825/AccessInvalidPointer.expected

@@ -10,7 +10,6 @@
 | deallocation.rs:95:5:95:31 | ...::write::<...> | deallocation.rs:70:3:70:21 | ...::dealloc | deallocation.rs:95:5:95:31 | ...::write::<...> | This operation dereferences a pointer that may be $@. | deallocation.rs:70:3:70:21 | ...::dealloc | invalid |
 | deallocation.rs:115:13:115:18 | my_ptr | deallocation.rs:112:3:112:12 | ...::free | deallocation.rs:115:13:115:18 | my_ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:112:3:112:12 | ...::free | invalid |
 | deallocation.rs:130:14:130:15 | p1 | deallocation.rs:123:23:123:40 | ...::dangling | deallocation.rs:130:14:130:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:123:23:123:40 | ...::dangling | invalid |
-| deallocation.rs:130:14:130:15 | p1 | deallocation.rs:123:23:123:40 | ...::dangling | deallocation.rs:130:14:130:15 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:123:23:123:40 | ...::dangling | invalid |
 | deallocation.rs:131:14:131:15 | p2 | deallocation.rs:124:21:124:42 | ...::dangling_mut | deallocation.rs:131:14:131:15 | p2 | This operation dereferences a pointer that may be $@. | deallocation.rs:124:21:124:42 | ...::dangling_mut | invalid |
 | deallocation.rs:132:14:132:15 | p3 | deallocation.rs:125:23:125:36 | ...::null | deallocation.rs:132:14:132:15 | p3 | This operation dereferences a pointer that may be $@. | deallocation.rs:125:23:125:36 | ...::null | invalid |
 | deallocation.rs:163:13:163:15 | ptr | deallocation.rs:159:9:159:26 | ...::null_mut | deallocation.rs:163:13:163:15 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:159:9:159:26 | ...::null_mut | invalid |
@@ -27,8 +26,6 @@
 | deallocation.rs:210:7:210:9 | ptr | deallocation.rs:207:9:207:26 | ...::null_mut | deallocation.rs:210:7:210:9 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:207:9:207:26 | ...::null_mut | invalid |
 | deallocation.rs:226:13:226:21 | const_ptr | deallocation.rs:219:15:219:32 | ...::null_mut | deallocation.rs:226:13:226:21 | const_ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:219:15:219:32 | ...::null_mut | invalid |
 | deallocation.rs:274:15:274:16 | p1 | deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:274:15:274:16 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:270:3:270:25 | ...::drop_in_place | invalid |
-| deallocation.rs:274:15:274:16 | p1 | deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:274:15:274:16 | p1 | This operation dereferences a pointer that may be $@. | deallocation.rs:270:3:270:25 | ...::drop_in_place | invalid |
-| deallocation.rs:342:18:342:20 | ptr | deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:342:18:342:20 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:336:3:336:25 | ...::drop_in_place | invalid |
 | deallocation.rs:342:18:342:20 | ptr | deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:342:18:342:20 | ptr | This operation dereferences a pointer that may be $@. | deallocation.rs:336:3:336:25 | ...::drop_in_place | invalid |
 edges
 | deallocation.rs:20:3:20:21 | ...::dealloc | deallocation.rs:20:23:20:24 | [post] m1 | provenance | Src:MaD:3 MaD:3 |
@@ -49,7 +46,6 @@ edges
 | deallocation.rs:112:14:112:40 | [post] my_ptr as ... | deallocation.rs:115:13:115:18 | my_ptr | provenance |  |
 | deallocation.rs:123:6:123:7 | p1 | deallocation.rs:130:14:130:15 | p1 | provenance |  |
 | deallocation.rs:123:23:123:40 | ...::dangling | deallocation.rs:123:23:123:42 | ...::dangling(...) | provenance | Src:MaD:4 MaD:4 |
-| deallocation.rs:123:23:123:40 | ...::dangling | deallocation.rs:123:23:123:42 | ...::dangling(...) | provenance | Src:MaD:4 MaD:4 |
 | deallocation.rs:123:23:123:42 | ...::dangling(...) | deallocation.rs:123:6:123:7 | p1 | provenance |  |
 | deallocation.rs:124:6:124:7 | p2 | deallocation.rs:131:14:131:15 | p2 | provenance |  |
 | deallocation.rs:124:21:124:42 | ...::dangling_mut | deallocation.rs:124:21:124:44 | ...::dangling_mut(...) | provenance | Src:MaD:5 MaD:5 |
@@ -83,10 +79,8 @@ edges
 | deallocation.rs:219:15:219:32 | ...::null_mut | deallocation.rs:219:15:219:34 | ...::null_mut(...) | provenance | Src:MaD:8 MaD:8 |
 | deallocation.rs:219:15:219:34 | ...::null_mut(...) | deallocation.rs:219:3:219:11 | const_ptr | provenance |  |
 | deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:270:27:270:28 | [post] p1 | provenance | Src:MaD:6 MaD:6 |
-| deallocation.rs:270:3:270:25 | ...::drop_in_place | deallocation.rs:270:27:270:28 | [post] p1 | provenance | Src:MaD:6 MaD:6 |
 | deallocation.rs:270:27:270:28 | [post] p1 | deallocation.rs:274:15:274:16 | p1 | provenance |  |
 | deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:336:27:336:29 | [post] ptr | provenance | Src:MaD:6 MaD:6 |
-| deallocation.rs:336:3:336:25 | ...::drop_in_place | deallocation.rs:336:27:336:29 | [post] ptr | provenance | Src:MaD:6 MaD:6 |
 | deallocation.rs:336:27:336:29 | [post] ptr | deallocation.rs:342:18:342:20 | ptr | provenance |  |
 models
 | 1 | Sink: core::ptr::read; Argument[0]; pointer-access |
@@ -120,7 +114,6 @@ nodes
 | deallocation.rs:115:13:115:18 | my_ptr | semmle.label | my_ptr |
 | deallocation.rs:123:6:123:7 | p1 | semmle.label | p1 |
 | deallocation.rs:123:23:123:40 | ...::dangling | semmle.label | ...::dangling |
-| deallocation.rs:123:23:123:40 | ...::dangling | semmle.label | ...::dangling |
 | deallocation.rs:123:23:123:42 | ...::dangling(...) | semmle.label | ...::dangling(...) |
 | deallocation.rs:124:6:124:7 | p2 | semmle.label | p2 |
 | deallocation.rs:124:21:124:42 | ...::dangling_mut | semmle.label | ...::dangling_mut |
@@ -160,11 +153,9 @@ nodes
 | deallocation.rs:219:15:219:34 | ...::null_mut(...) | semmle.label | ...::null_mut(...) |
 | deallocation.rs:226:13:226:21 | const_ptr | semmle.label | const_ptr |
 | deallocation.rs:270:3:270:25 | ...::drop_in_place | semmle.label | ...::drop_in_place |
-| deallocation.rs:270:3:270:25 | ...::drop_in_place | semmle.label | ...::drop_in_place |
 | deallocation.rs:270:27:270:28 | [post] p1 | semmle.label | [post] p1 |
 | deallocation.rs:274:15:274:16 | p1 | semmle.label | p1 |
 | deallocation.rs:336:3:336:25 | ...::drop_in_place | semmle.label | ...::drop_in_place |
-| deallocation.rs:336:3:336:25 | ...::drop_in_place | semmle.label | ...::drop_in_place |
 | deallocation.rs:336:27:336:29 | [post] ptr | semmle.label | [post] ptr |
 | deallocation.rs:342:18:342:20 | ptr | semmle.label | ptr |
 subpaths

shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll

@@ -662,16 +662,40 @@ module Make<
       unsupportedCallable(callable, _, _, _)
     }
 
+    private predicate isRelevantSource(
+      SourceElement e, string output, string kind, Provenance provenance, string model
+    ) {
+      e.isSource(output, kind, provenance, model) and
+      (
+        provenance.isManual()
+        or
+        provenance.isGenerated() and
+        not exists(Provenance p | p.isManual() and e.isSource(_, kind, p, _))
+      )
+    }
+
+    private predicate isRelevantSink(
+      SinkElement e, string input, string kind, Provenance provenance, string model
+    ) {
+      e.isSink(input, kind, provenance, model) and
+      (
+        provenance.isManual()
+        or
+        provenance.isGenerated() and
+        not exists(Provenance p | p.isManual() and e.isSink(_, kind, p, _))
+      )
+    }
+
     private predicate summarySpec(string spec) {
       exists(SummarizedCallable c |
         c.propagatesFlow(spec, _, _, _)
         or
         c.propagatesFlow(_, spec, _, _)
       )
       or
-      any(SourceElement s).isSource(spec, _, _, _)
+      isRelevantSource(_, spec, _, _, _)
       or
-      any(SinkElement s).isSink(spec, _, _, _)
+      isRelevantSink(_, spec, _, _, _)
     }
 
     import AccessPathSyntax::AccessPath<summarySpec/1>
@@ -1034,7 +1058,7 @@ module Make<
       SourceElement source, SummaryComponentStack s, string kind, string model
     ) {
       exists(string outSpec |
-        source.isSource(outSpec, kind, _, model) and
+        isRelevantSource(source, outSpec, kind, _, model) and
         External::interpretSpec(outSpec, s)
       )
     }
@@ -1057,7 +1081,7 @@ module Make<
       SinkElement sink, SummaryComponentStack s, string kind, string model
     ) {
       exists(string inSpec |
-        sink.isSink(inSpec, kind, _, model) and
+        isRelevantSink(sink, inSpec, kind, _, model) and
         External::interpretSpec(inSpec, s)
       )
     }