Merge pull request #16458 from owen-mc/go/fix-mad-for-builtin-functions

Go: fix `hasQualifiedName` and models-as-data for built-in functions
github · May 9, 2024 · 526204d · 526204d
2 parents 4dfcdbc + 4f10cb5
commit 526204d
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 3 deletions.
diff --git a/go/ql/lib/change-notes/2024-05-08-builtin-functions-no-package.md b/go/ql/lib/change-notes/2024-05-08-builtin-functions-no-package.md
@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* Fixed a bug that stopped built-in functions from being referenced using the predicate `hasQualifiedName` because technically they do not belong to any package. Now you can use the empty string as the package, e.g. `f.hasQualifiedName("", "len")`.
+* Fixed a bug that stopped data flow models for built-in functions from having any effect because the package "" was not parsed correctly.
diff --git a/go/ql/lib/semmle/go/Scopes.qll b/go/ql/lib/semmle/go/Scopes.qll
@@ -103,7 +103,11 @@ class Entity extends @object {
    */
   pragma[nomagic]
   predicate hasQualifiedName(string pkg, string name) {
-    pkg = this.getPackage().getPath() and
+    (
+      pkg = this.getPackage().getPath()
+      or
+      not exists(this.getPackage()) and pkg = ""
+    ) and
     name = this.getName()
   }
 

diff --git a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll
@@ -271,6 +271,8 @@ private string interpretPackage(string p) {
     then result = package(p.regexpCapture(r, 1), p.regexpCapture(r, 4))
     else result = package(p, "")
   )
+  or
+  p = "" and result = ""
 }
 
 /** Gets the source/sink/summary element corresponding to the supplied parameters. */

diff --git a/go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.ql b/go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.ql
@@ -1,5 +1,7 @@
 import go
 
 from DataFlow::Node nd, DataFlow::Node succ
-where DataFlow::localFlowStep(nd, succ)
+where
+  DataFlow::localFlowStep(nd, succ) and
+  (exists(nd.getFile()) or exists(succ.getFile()))
 select nd, succ
diff --git a/go/ql/test/library-tests/semmle/go/dataflow/PromotedFields/LocalFlowStep.ql b/go/ql/test/library-tests/semmle/go/dataflow/PromotedFields/LocalFlowStep.ql
@@ -1,5 +1,7 @@
 import go
 
 from DataFlow::Node nd, DataFlow::Node succ
-where DataFlow::localFlowStep(nd, succ)
+where
+  DataFlow::localFlowStep(nd, succ) and
+  (exists(nd.getFile()) or exists(succ.getFile()))
 select nd, succ
diff --git a/go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected b/go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected
@@ -13,6 +13,7 @@ edges
 | SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:13:25:13:31 | tainted | provenance |  |
 | SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:14:23:14:33 | slice expression | provenance |  |
 | SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:39:31:39:37 | tainted | provenance |  |
+| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:52:24:52:30 | tainted | provenance |  |
 | SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:53:21:53:28 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:68:31:68:37 | tainted | provenance |  |
 | SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:80:23:80:29 | tainted | provenance |  |
@@ -23,8 +24,12 @@ edges
 | SanitizingDoubleDash.go:14:23:14:33 | slice expression [array] | SanitizingDoubleDash.go:14:23:14:33 | slice expression | provenance |  |
 | SanitizingDoubleDash.go:39:14:39:44 | call to append | SanitizingDoubleDash.go:40:23:40:30 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:39:31:39:37 | tainted | SanitizingDoubleDash.go:39:14:39:44 | call to append | provenance | FunctionModel |
+| SanitizingDoubleDash.go:52:15:52:31 | slice literal [array] | SanitizingDoubleDash.go:53:21:53:28 | arrayLit [array] | provenance |  |
+| SanitizingDoubleDash.go:52:24:52:30 | tainted | SanitizingDoubleDash.go:52:15:52:31 | slice literal [array] | provenance |  |
 | SanitizingDoubleDash.go:53:14:53:35 | call to append | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | provenance |  |
+| SanitizingDoubleDash.go:53:14:53:35 | call to append [array] | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:53:21:53:28 | arrayLit | SanitizingDoubleDash.go:53:14:53:35 | call to append | provenance | FunctionModel |
+| SanitizingDoubleDash.go:53:21:53:28 | arrayLit [array] | SanitizingDoubleDash.go:53:14:53:35 | call to append [array] | provenance | MaD:28 |
 | SanitizingDoubleDash.go:68:14:68:38 | call to append | SanitizingDoubleDash.go:69:21:69:28 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:68:31:68:37 | tainted | SanitizingDoubleDash.go:68:14:68:38 | call to append | provenance | FunctionModel |
 | SanitizingDoubleDash.go:69:14:69:35 | call to append | SanitizingDoubleDash.go:70:23:70:30 | arrayLit | provenance |  |
@@ -39,6 +44,7 @@ edges
 | SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:111:37:111:43 | tainted | provenance |  |
 | SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:117:31:117:37 | tainted | provenance |  |
 | SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:123:31:123:37 | tainted | provenance |  |
+| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:128:24:128:30 | tainted | provenance |  |
 | SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:129:21:129:28 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:136:31:136:37 | tainted | provenance |  |
 | SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:142:31:142:37 | tainted | provenance |  |
@@ -62,8 +68,12 @@ edges
 | SanitizingDoubleDash.go:117:31:117:37 | tainted | SanitizingDoubleDash.go:117:14:117:44 | call to append | provenance | FunctionModel |
 | SanitizingDoubleDash.go:123:14:123:38 | call to append | SanitizingDoubleDash.go:124:24:124:31 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:123:31:123:37 | tainted | SanitizingDoubleDash.go:123:14:123:38 | call to append | provenance | FunctionModel |
+| SanitizingDoubleDash.go:128:15:128:31 | slice literal [array] | SanitizingDoubleDash.go:129:21:129:28 | arrayLit [array] | provenance |  |
+| SanitizingDoubleDash.go:128:24:128:30 | tainted | SanitizingDoubleDash.go:128:15:128:31 | slice literal [array] | provenance |  |
 | SanitizingDoubleDash.go:129:14:129:35 | call to append | SanitizingDoubleDash.go:130:24:130:31 | arrayLit | provenance |  |
+| SanitizingDoubleDash.go:129:14:129:35 | call to append [array] | SanitizingDoubleDash.go:130:24:130:31 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:129:21:129:28 | arrayLit | SanitizingDoubleDash.go:129:14:129:35 | call to append | provenance | FunctionModel |
+| SanitizingDoubleDash.go:129:21:129:28 | arrayLit [array] | SanitizingDoubleDash.go:129:14:129:35 | call to append [array] | provenance | MaD:28 |
 | SanitizingDoubleDash.go:136:14:136:38 | call to append | SanitizingDoubleDash.go:137:24:137:31 | arrayLit | provenance |  |
 | SanitizingDoubleDash.go:136:31:136:37 | tainted | SanitizingDoubleDash.go:136:14:136:38 | call to append | provenance | FunctionModel |
 | SanitizingDoubleDash.go:142:14:142:38 | call to append | SanitizingDoubleDash.go:143:21:143:28 | arrayLit | provenance |  |
@@ -95,8 +105,12 @@ nodes
 | SanitizingDoubleDash.go:39:14:39:44 | call to append | semmle.label | call to append |
 | SanitizingDoubleDash.go:39:31:39:37 | tainted | semmle.label | tainted |
 | SanitizingDoubleDash.go:40:23:40:30 | arrayLit | semmle.label | arrayLit |
+| SanitizingDoubleDash.go:52:15:52:31 | slice literal [array] | semmle.label | slice literal [array] |
+| SanitizingDoubleDash.go:52:24:52:30 | tainted | semmle.label | tainted |
 | SanitizingDoubleDash.go:53:14:53:35 | call to append | semmle.label | call to append |
+| SanitizingDoubleDash.go:53:14:53:35 | call to append [array] | semmle.label | call to append [array] |
 | SanitizingDoubleDash.go:53:21:53:28 | arrayLit | semmle.label | arrayLit |
+| SanitizingDoubleDash.go:53:21:53:28 | arrayLit [array] | semmle.label | arrayLit [array] |
 | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | semmle.label | arrayLit |
 | SanitizingDoubleDash.go:68:14:68:38 | call to append | semmle.label | call to append |
 | SanitizingDoubleDash.go:68:31:68:37 | tainted | semmle.label | tainted |
@@ -130,8 +144,12 @@ nodes
 | SanitizingDoubleDash.go:123:14:123:38 | call to append | semmle.label | call to append |
 | SanitizingDoubleDash.go:123:31:123:37 | tainted | semmle.label | tainted |
 | SanitizingDoubleDash.go:124:24:124:31 | arrayLit | semmle.label | arrayLit |
+| SanitizingDoubleDash.go:128:15:128:31 | slice literal [array] | semmle.label | slice literal [array] |
+| SanitizingDoubleDash.go:128:24:128:30 | tainted | semmle.label | tainted |
 | SanitizingDoubleDash.go:129:14:129:35 | call to append | semmle.label | call to append |
+| SanitizingDoubleDash.go:129:14:129:35 | call to append [array] | semmle.label | call to append [array] |
 | SanitizingDoubleDash.go:129:21:129:28 | arrayLit | semmle.label | arrayLit |
+| SanitizingDoubleDash.go:129:21:129:28 | arrayLit [array] | semmle.label | arrayLit [array] |
 | SanitizingDoubleDash.go:130:24:130:31 | arrayLit | semmle.label | arrayLit |
 | SanitizingDoubleDash.go:136:14:136:38 | call to append | semmle.label | call to append |
 | SanitizingDoubleDash.go:136:31:136:37 | tainted | semmle.label | tainted |