Skip to content

Commit 145873f

Browse files
owen-mcowen-mc
authored
Merge pull request github#16413 from owen-mc/go/fix-builtin-models
Go: fix models for built-in functions
2 parents 04c0475 + 827d15a commit 145873f

File tree

71 files changed

+844
-411
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+844
-411
lines changed

‎go/ql/lib/change-notes/2024-05-09-model-append-copy-max-min.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Converted the models for the built-in functions `append`, `copy`, `max` and `min` to value flow and Models-as-Data.

‎go/ql/lib/ext/builtin.model.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,7 @@ extensions:
44
extensible: summaryModel
55
data:
66
- ["", "", False, "append", "", "", "Argument[0].ArrayElement", "ReturnValue.ArrayElement", "value", "manual"]
7-
- ["", "", False, "append", "", "", "Argument[1]", "ReturnValue.ArrayElement", "value", "manual"]
7+
- ["", "", False, "append", "", "", "Argument[1].ArrayElement", "ReturnValue.ArrayElement", "value", "manual"]
8+
- ["", "", False, "copy", "", "", "Argument[1].ArrayElement", "Argument[0].ArrayElement", "value", "manual"]
9+
- ["", "", False, "max", "", "", "Argument[0..1000]", "ReturnValue", "value", "manual"]
10+
- ["", "", False, "min", "", "", "Argument[0..1000]", "ReturnValue", "value", "manual"]

‎go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ predicate referenceStep(DataFlow::Node pred, DataFlow::Node succ) {
140140
*/
141141
predicate elementWriteStep(DataFlow::Node pred, DataFlow::Node succ) {
142142
any(DataFlow::Write w).writesElement(succ.(DataFlow::PostUpdateNode).getPreUpdateNode(), _, pred)
143+
or
144+
FlowSummaryImpl::Private::Steps::summaryStoreStep(pred.(DataFlowPrivate::FlowSummaryNode)
145+
.getSummaryNode(), any(DataFlow::Content c | c instanceof DataFlow::ArrayContent),
146+
succ.(DataFlowPrivate::FlowSummaryNode).getSummaryNode())
143147
}
144148

145149
/** Holds if taint flows from `pred` to `succ` via a field read. */

‎go/ql/lib/semmle/go/frameworks/Stdlib.qll

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -44,58 +44,6 @@ import semmle.go.frameworks.stdlib.TextTabwriter
4444
import semmle.go.frameworks.stdlib.TextTemplate
4545
import semmle.go.frameworks.stdlib.Unsafe
4646

47-
// These are modeled using TaintTracking::FunctionModel because they doesn't have real type signatures,
48-
// and therefore currently have an InvalidType, not a SignatureType, which breaks Models as Data.
49-
/**
50-
* A model of the built-in `append` function, which propagates taint from its arguments to its
51-
* result.
52-
*/
53-
private class AppendFunction extends TaintTracking::FunctionModel {
54-
AppendFunction() { this = Builtin::append() }
55-
56-
override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
57-
inp.isParameter(_) and outp.isResult()
58-
}
59-
}
60-
61-
/**
62-
* A model of the built-in `copy` function, which propagates taint from its second argument
63-
* to its first.
64-
*/
65-
private class CopyFunction extends TaintTracking::FunctionModel {
66-
CopyFunction() { this = Builtin::copy() }
67-
68-
override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
69-
inp.isParameter(1) and outp.isParameter(0)
70-
}
71-
}
72-
73-
/**
74-
* A model of the built-in `min` function, which computes the smallest value of a fixed number of
75-
* arguments of ordered types. There is at least one argument and "ordered types" includes e.g.
76-
* strings, so we care about data flow through `min`.
77-
*/
78-
private class MinFunction extends DataFlow::FunctionModel {
79-
MinFunction() { this = Builtin::min_() }
80-
81-
override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp) {
82-
inp.isParameter(_) and outp.isResult()
83-
}
84-
}
85-
86-
/**
87-
* A model of the built-in `max` function, which computes the largest value of a fixed number of
88-
* arguments of ordered types. There is at least one argument and "ordered types" includes e.g.
89-
* strings, so we care about data flow through `max`.
90-
*/
91-
private class MaxFunction extends DataFlow::FunctionModel {
92-
MaxFunction() { this = Builtin::max_() }
93-
94-
override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp) {
95-
inp.isParameter(_) and outp.isResult()
96-
}
97-
}
98-
9947
/** Provides a class for modeling functions which convert strings into integers. */
10048
module IntegerParser {
10149
/**

‎go/ql/test/experimental/CWE-090/LDAPInjection.expected

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
edges
2-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:671 |
3-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:671 |
4-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:671 |
5-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:671 |
6-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:671 |
7-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:671 |
8-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:671 |
9-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:671 |
10-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:671 |
11-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:671 |
12-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:671 |
13-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:671 |
14-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:671 |
15-
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:671 |
2+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:674 |
3+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:674 |
4+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:674 |
5+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:674 |
6+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:674 |
7+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:674 |
8+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:674 |
9+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:674 |
10+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:674 |
11+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:674 |
12+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:674 |
13+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:674 |
14+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:674 |
15+
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:674 |
1616
| LDAPInjection.go:62:3:62:33 | slice literal [array] | LDAPInjection.go:62:3:62:33 | slice literal | provenance | |
1717
| LDAPInjection.go:62:24:62:32 | untrusted | LDAPInjection.go:62:3:62:33 | slice literal [array] | provenance | |
1818
| LDAPInjection.go:69:3:69:33 | slice literal [array] | LDAPInjection.go:69:3:69:33 | slice literal | provenance | |

‎go/ql/test/experimental/CWE-203/Timing.expected

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
edges
2-
| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | MaD:652 |
2+
| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | MaD:655 |
33
| timing.go:15:18:15:45 | call to Get | timing.go:17:31:17:42 | headerSecret | provenance | |
4-
| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | MaD:652 |
4+
| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | MaD:655 |
55
| timing.go:28:18:28:45 | call to Get | timing.go:30:47:30:58 | headerSecret | provenance | |
6-
| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | MaD:652 |
6+
| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | MaD:655 |
77
| timing.go:41:18:41:45 | call to Get | timing.go:42:25:42:36 | headerSecret | provenance | |
88
nodes
99
| timing.go:15:18:15:27 | selection of Header | semmle.label | selection of Header |

‎go/ql/test/experimental/CWE-287/ImproperLdapAuth.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
edges
2-
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | MaD:732 |
2+
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | MaD:735 |
33
| ImproperLdapAuth.go:18:18:18:32 | call to Query | ImproperLdapAuth.go:28:23:28:34 | bindPassword | provenance | |
44
| ImproperLdapAuth.go:87:18:87:19 | "" | ImproperLdapAuth.go:97:23:97:34 | bindPassword | provenance | |
55
nodes

‎go/ql/test/experimental/CWE-369/DivideByZero.expected

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,24 @@
11
edges
2-
| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | MaD:732 |
2+
| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | MaD:735 |
33
| DivideByZero.go:10:12:10:24 | call to Query | DivideByZero.go:11:27:11:32 | param1 | provenance | |
44
| DivideByZero.go:11:2:11:33 | ... := ...[0] | DivideByZero.go:12:16:12:20 | value | provenance | |
55
| DivideByZero.go:11:27:11:32 | param1 | DivideByZero.go:11:2:11:33 | ... := ...[0] | provenance | |
6-
| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | MaD:732 |
6+
| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | MaD:735 |
77
| DivideByZero.go:17:12:17:24 | call to Query | DivideByZero.go:18:11:18:24 | type conversion | provenance | |
88
| DivideByZero.go:18:11:18:24 | type conversion | DivideByZero.go:19:16:19:20 | value | provenance | |
9-
| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | MaD:732 |
9+
| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | MaD:735 |
1010
| DivideByZero.go:24:12:24:24 | call to Query | DivideByZero.go:25:31:25:36 | param1 | provenance | |
1111
| DivideByZero.go:25:2:25:45 | ... := ...[0] | DivideByZero.go:26:16:26:20 | value | provenance | |
1212
| DivideByZero.go:25:31:25:36 | param1 | DivideByZero.go:25:2:25:45 | ... := ...[0] | provenance | |
13-
| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | MaD:732 |
13+
| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | MaD:735 |
1414
| DivideByZero.go:31:12:31:24 | call to Query | DivideByZero.go:32:33:32:38 | param1 | provenance | |
1515
| DivideByZero.go:32:2:32:43 | ... := ...[0] | DivideByZero.go:33:16:33:20 | value | provenance | |
1616
| DivideByZero.go:32:33:32:38 | param1 | DivideByZero.go:32:2:32:43 | ... := ...[0] | provenance | |
17-
| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | MaD:732 |
17+
| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | MaD:735 |
1818
| DivideByZero.go:38:12:38:24 | call to Query | DivideByZero.go:39:32:39:37 | param1 | provenance | |
1919
| DivideByZero.go:39:2:39:46 | ... := ...[0] | DivideByZero.go:40:16:40:20 | value | provenance | |
2020
| DivideByZero.go:39:32:39:37 | param1 | DivideByZero.go:39:2:39:46 | ... := ...[0] | provenance | |
21-
| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | MaD:732 |
21+
| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | MaD:735 |
2222
| DivideByZero.go:54:12:54:24 | call to Query | DivideByZero.go:55:11:55:24 | type conversion | provenance | |
2323
| DivideByZero.go:55:11:55:24 | type conversion | DivideByZero.go:57:17:57:21 | value | provenance | |
2424
nodes

‎go/ql/test/experimental/CWE-522-DecompressionBombs/DecompressionBombs.expected

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
edges
2-
| test.go:59:16:59:44 | call to FormValue | test.go:128:20:128:27 | definition of filename | provenance | Src:MaD:667 |
2+
| test.go:59:16:59:44 | call to FormValue | test.go:128:20:128:27 | definition of filename | provenance | Src:MaD:670 |
33
| test.go:60:15:60:26 | selection of Body | test.go:158:19:158:22 | definition of file | provenance | |
44
| test.go:61:24:61:35 | selection of Body | test.go:169:28:169:31 | definition of file | provenance | |
55
| test.go:62:13:62:24 | selection of Body | test.go:181:17:181:20 | definition of file | provenance | |
@@ -34,18 +34,18 @@ edges
3434
| test.go:145:12:145:19 | call to Open | test.go:147:37:147:38 | rc | provenance | |
3535
| test.go:158:19:158:22 | definition of file | test.go:159:25:159:28 | file | provenance | |
3636
| test.go:159:2:159:29 | ... := ...[0] | test.go:160:48:160:52 | file1 | provenance | |
37-
| test.go:159:25:159:28 | file | test.go:159:2:159:29 | ... := ...[0] | provenance | MaD:544 |
37+
| test.go:159:25:159:28 | file | test.go:159:2:159:29 | ... := ...[0] | provenance | MaD:547 |
3838
| test.go:160:2:160:69 | ... := ...[0] | test.go:163:26:163:29 | file | provenance | |
3939
| test.go:160:32:160:53 | call to NewReader | test.go:160:2:160:69 | ... := ...[0] | provenance | |
40-
| test.go:160:48:160:52 | file1 | test.go:160:32:160:53 | call to NewReader | provenance | MaD:40 |
40+
| test.go:160:48:160:52 | file1 | test.go:160:32:160:53 | call to NewReader | provenance | MaD:43 |
4141
| test.go:163:3:163:36 | ... := ...[0] | test.go:164:36:164:51 | fileReaderCloser | provenance | |
4242
| test.go:163:26:163:29 | file | test.go:163:3:163:36 | ... := ...[0] | provenance | MaD:8 |
4343
| test.go:169:28:169:31 | definition of file | test.go:170:25:170:28 | file | provenance | |
4444
| test.go:170:2:170:29 | ... := ...[0] | test.go:171:57:171:61 | file2 | provenance | |
45-
| test.go:170:25:170:28 | file | test.go:170:2:170:29 | ... := ...[0] | provenance | MaD:544 |
45+
| test.go:170:25:170:28 | file | test.go:170:2:170:29 | ... := ...[0] | provenance | MaD:547 |
4646
| test.go:171:2:171:78 | ... := ...[0] | test.go:175:26:175:29 | file | provenance | |
4747
| test.go:171:41:171:62 | call to NewReader | test.go:171:2:171:78 | ... := ...[0] | provenance | |
48-
| test.go:171:57:171:61 | file2 | test.go:171:41:171:62 | call to NewReader | provenance | MaD:40 |
48+
| test.go:171:57:171:61 | file2 | test.go:171:41:171:62 | call to NewReader | provenance | MaD:43 |
4949
| test.go:175:26:175:29 | file | test.go:175:26:175:36 | call to Open | provenance | |
5050
| test.go:175:26:175:36 | call to Open | test.go:176:36:176:51 | fileReaderCloser | provenance | |
5151
| test.go:181:17:181:20 | definition of file | test.go:184:41:184:44 | file | provenance | |

‎go/ql/test/experimental/CWE-74/DsnInjection.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
edges
2-
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name | provenance | Src:MaD:667 |
3-
| Dsn.go:49:11:49:106 | []type{args} [array] | Dsn.go:49:11:49:106 | call to Sprintf | provenance | MaD:242 |
2+
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name | provenance | Src:MaD:670 |
3+
| Dsn.go:49:11:49:106 | []type{args} [array] | Dsn.go:49:11:49:106 | call to Sprintf | provenance | MaD:245 |
44
| Dsn.go:49:11:49:106 | call to Sprintf | Dsn.go:50:29:50:33 | dbDSN | provenance | |
55
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | []type{args} [array] | provenance | |
66
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | call to Sprintf | provenance | FunctionModel |

0 commit comments

Comments
 (0)