Skip to content

Commit 526204d

Browse files
authored
Merge pull request github#16458 from owen-mc/go/fix-mad-for-builtin-functions
Go: fix `hasQualifiedName` and models-as-data for built-in functions
2 parents 4dfcdbc + 4f10cb5 commit 526204d

File tree

6 files changed

+36
-3
lines changed

6 files changed

+36
-3
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* 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")`.
5+
* Fixed a bug that stopped data flow models for built-in functions from having any effect because the package "" was not parsed correctly.

go/ql/lib/semmle/go/Scopes.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,11 @@ class Entity extends @object {
103103
*/
104104
pragma[nomagic]
105105
predicate hasQualifiedName(string pkg, string name) {
106-
pkg = this.getPackage().getPath() and
106+
(
107+
pkg = this.getPackage().getPath()
108+
or
109+
not exists(this.getPackage()) and pkg = ""
110+
) and
107111
name = this.getName()
108112
}
109113

go/ql/lib/semmle/go/dataflow/ExternalFlow.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,8 @@ private string interpretPackage(string p) {
271271
then result = package(p.regexpCapture(r, 1), p.regexpCapture(r, 4))
272272
else result = package(p, "")
273273
)
274+
or
275+
p = "" and result = ""
274276
}
275277

276278
/** Gets the source/sink/summary element corresponding to the supplied parameters. */
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import go
22

33
from DataFlow::Node nd, DataFlow::Node succ
4-
where DataFlow::localFlowStep(nd, succ)
4+
where
5+
DataFlow::localFlowStep(nd, succ) and
6+
(exists(nd.getFile()) or exists(succ.getFile()))
57
select nd, succ
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import go
22

33
from DataFlow::Node nd, DataFlow::Node succ
4-
where DataFlow::localFlowStep(nd, succ)
4+
where
5+
DataFlow::localFlowStep(nd, succ) and
6+
(exists(nd.getFile()) or exists(succ.getFile()))
57
select nd, succ

go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ edges
1313
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:13:25:13:31 | tainted | provenance | |
1414
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:14:23:14:33 | slice expression | provenance | |
1515
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:39:31:39:37 | tainted | provenance | |
16+
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:52:24:52:30 | tainted | provenance | |
1617
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:53:21:53:28 | arrayLit | provenance | |
1718
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:68:31:68:37 | tainted | provenance | |
1819
| SanitizingDoubleDash.go:9:13:9:27 | call to Query | SanitizingDoubleDash.go:80:23:80:29 | tainted | provenance | |
@@ -23,8 +24,12 @@ edges
2324
| SanitizingDoubleDash.go:14:23:14:33 | slice expression [array] | SanitizingDoubleDash.go:14:23:14:33 | slice expression | provenance | |
2425
| SanitizingDoubleDash.go:39:14:39:44 | call to append | SanitizingDoubleDash.go:40:23:40:30 | arrayLit | provenance | |
2526
| SanitizingDoubleDash.go:39:31:39:37 | tainted | SanitizingDoubleDash.go:39:14:39:44 | call to append | provenance | FunctionModel |
27+
| SanitizingDoubleDash.go:52:15:52:31 | slice literal [array] | SanitizingDoubleDash.go:53:21:53:28 | arrayLit [array] | provenance | |
28+
| SanitizingDoubleDash.go:52:24:52:30 | tainted | SanitizingDoubleDash.go:52:15:52:31 | slice literal [array] | provenance | |
2629
| SanitizingDoubleDash.go:53:14:53:35 | call to append | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | provenance | |
30+
| SanitizingDoubleDash.go:53:14:53:35 | call to append [array] | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | provenance | |
2731
| SanitizingDoubleDash.go:53:21:53:28 | arrayLit | SanitizingDoubleDash.go:53:14:53:35 | call to append | provenance | FunctionModel |
32+
| SanitizingDoubleDash.go:53:21:53:28 | arrayLit [array] | SanitizingDoubleDash.go:53:14:53:35 | call to append [array] | provenance | MaD:28 |
2833
| SanitizingDoubleDash.go:68:14:68:38 | call to append | SanitizingDoubleDash.go:69:21:69:28 | arrayLit | provenance | |
2934
| SanitizingDoubleDash.go:68:31:68:37 | tainted | SanitizingDoubleDash.go:68:14:68:38 | call to append | provenance | FunctionModel |
3035
| SanitizingDoubleDash.go:69:14:69:35 | call to append | SanitizingDoubleDash.go:70:23:70:30 | arrayLit | provenance | |
@@ -39,6 +44,7 @@ edges
3944
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:111:37:111:43 | tainted | provenance | |
4045
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:117:31:117:37 | tainted | provenance | |
4146
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:123:31:123:37 | tainted | provenance | |
47+
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:128:24:128:30 | tainted | provenance | |
4248
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:129:21:129:28 | arrayLit | provenance | |
4349
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:136:31:136:37 | tainted | provenance | |
4450
| SanitizingDoubleDash.go:92:13:92:27 | call to Query | SanitizingDoubleDash.go:142:31:142:37 | tainted | provenance | |
@@ -62,8 +68,12 @@ edges
6268
| SanitizingDoubleDash.go:117:31:117:37 | tainted | SanitizingDoubleDash.go:117:14:117:44 | call to append | provenance | FunctionModel |
6369
| SanitizingDoubleDash.go:123:14:123:38 | call to append | SanitizingDoubleDash.go:124:24:124:31 | arrayLit | provenance | |
6470
| SanitizingDoubleDash.go:123:31:123:37 | tainted | SanitizingDoubleDash.go:123:14:123:38 | call to append | provenance | FunctionModel |
71+
| SanitizingDoubleDash.go:128:15:128:31 | slice literal [array] | SanitizingDoubleDash.go:129:21:129:28 | arrayLit [array] | provenance | |
72+
| SanitizingDoubleDash.go:128:24:128:30 | tainted | SanitizingDoubleDash.go:128:15:128:31 | slice literal [array] | provenance | |
6573
| SanitizingDoubleDash.go:129:14:129:35 | call to append | SanitizingDoubleDash.go:130:24:130:31 | arrayLit | provenance | |
74+
| SanitizingDoubleDash.go:129:14:129:35 | call to append [array] | SanitizingDoubleDash.go:130:24:130:31 | arrayLit | provenance | |
6675
| SanitizingDoubleDash.go:129:21:129:28 | arrayLit | SanitizingDoubleDash.go:129:14:129:35 | call to append | provenance | FunctionModel |
76+
| SanitizingDoubleDash.go:129:21:129:28 | arrayLit [array] | SanitizingDoubleDash.go:129:14:129:35 | call to append [array] | provenance | MaD:28 |
6777
| SanitizingDoubleDash.go:136:14:136:38 | call to append | SanitizingDoubleDash.go:137:24:137:31 | arrayLit | provenance | |
6878
| SanitizingDoubleDash.go:136:31:136:37 | tainted | SanitizingDoubleDash.go:136:14:136:38 | call to append | provenance | FunctionModel |
6979
| SanitizingDoubleDash.go:142:14:142:38 | call to append | SanitizingDoubleDash.go:143:21:143:28 | arrayLit | provenance | |
@@ -95,8 +105,12 @@ nodes
95105
| SanitizingDoubleDash.go:39:14:39:44 | call to append | semmle.label | call to append |
96106
| SanitizingDoubleDash.go:39:31:39:37 | tainted | semmle.label | tainted |
97107
| SanitizingDoubleDash.go:40:23:40:30 | arrayLit | semmle.label | arrayLit |
108+
| SanitizingDoubleDash.go:52:15:52:31 | slice literal [array] | semmle.label | slice literal [array] |
109+
| SanitizingDoubleDash.go:52:24:52:30 | tainted | semmle.label | tainted |
98110
| SanitizingDoubleDash.go:53:14:53:35 | call to append | semmle.label | call to append |
111+
| SanitizingDoubleDash.go:53:14:53:35 | call to append [array] | semmle.label | call to append [array] |
99112
| SanitizingDoubleDash.go:53:21:53:28 | arrayLit | semmle.label | arrayLit |
113+
| SanitizingDoubleDash.go:53:21:53:28 | arrayLit [array] | semmle.label | arrayLit [array] |
100114
| SanitizingDoubleDash.go:54:23:54:30 | arrayLit | semmle.label | arrayLit |
101115
| SanitizingDoubleDash.go:68:14:68:38 | call to append | semmle.label | call to append |
102116
| SanitizingDoubleDash.go:68:31:68:37 | tainted | semmle.label | tainted |
@@ -130,8 +144,12 @@ nodes
130144
| SanitizingDoubleDash.go:123:14:123:38 | call to append | semmle.label | call to append |
131145
| SanitizingDoubleDash.go:123:31:123:37 | tainted | semmle.label | tainted |
132146
| SanitizingDoubleDash.go:124:24:124:31 | arrayLit | semmle.label | arrayLit |
147+
| SanitizingDoubleDash.go:128:15:128:31 | slice literal [array] | semmle.label | slice literal [array] |
148+
| SanitizingDoubleDash.go:128:24:128:30 | tainted | semmle.label | tainted |
133149
| SanitizingDoubleDash.go:129:14:129:35 | call to append | semmle.label | call to append |
150+
| SanitizingDoubleDash.go:129:14:129:35 | call to append [array] | semmle.label | call to append [array] |
134151
| SanitizingDoubleDash.go:129:21:129:28 | arrayLit | semmle.label | arrayLit |
152+
| SanitizingDoubleDash.go:129:21:129:28 | arrayLit [array] | semmle.label | arrayLit [array] |
135153
| SanitizingDoubleDash.go:130:24:130:31 | arrayLit | semmle.label | arrayLit |
136154
| SanitizingDoubleDash.go:136:14:136:38 | call to append | semmle.label | call to append |
137155
| SanitizingDoubleDash.go:136:31:136:37 | tainted | semmle.label | tainted |

0 commit comments

Comments
 (0)