Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go: fix models for built-in functions #16413

Merged
merged 16 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Converted the models for the built-in functions `append`, `copy`, `max` and `min` to value flow and Models-as-Data.
5 changes: 4 additions & 1 deletion go/ql/lib/ext/builtin.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ extensions:
extensible: summaryModel
data:
- ["", "", False, "append", "", "", "Argument[0].ArrayElement", "ReturnValue.ArrayElement", "value", "manual"]
- ["", "", False, "append", "", "", "Argument[1]", "ReturnValue.ArrayElement", "value", "manual"]
- ["", "", False, "append", "", "", "Argument[1].ArrayElement", "ReturnValue.ArrayElement", "value", "manual"]
- ["", "", False, "copy", "", "", "Argument[1].ArrayElement", "Argument[0].ArrayElement", "value", "manual"]
- ["", "", False, "max", "", "", "Argument[0..1000]", "ReturnValue", "value", "manual"]
- ["", "", False, "min", "", "", "Argument[0..1000]", "ReturnValue", "value", "manual"]
4 changes: 4 additions & 0 deletions go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ predicate referenceStep(DataFlow::Node pred, DataFlow::Node succ) {
*/
predicate elementWriteStep(DataFlow::Node pred, DataFlow::Node succ) {
any(DataFlow::Write w).writesElement(succ.(DataFlow::PostUpdateNode).getPreUpdateNode(), _, pred)
or
FlowSummaryImpl::Private::Steps::summaryStoreStep(pred.(DataFlowPrivate::FlowSummaryNode)
.getSummaryNode(), any(DataFlow::Content c | c instanceof DataFlow::ArrayContent),
succ.(DataFlowPrivate::FlowSummaryNode).getSummaryNode())
}

/** Holds if taint flows from `pred` to `succ` via a field read. */
Expand Down
52 changes: 0 additions & 52 deletions go/ql/lib/semmle/go/frameworks/Stdlib.qll
Original file line number Diff line number Diff line change
Expand Up @@ -44,58 +44,6 @@ import semmle.go.frameworks.stdlib.TextTabwriter
import semmle.go.frameworks.stdlib.TextTemplate
import semmle.go.frameworks.stdlib.Unsafe

// These are modeled using TaintTracking::FunctionModel because they doesn't have real type signatures,
// and therefore currently have an InvalidType, not a SignatureType, which breaks Models as Data.
/**
* A model of the built-in `append` function, which propagates taint from its arguments to its
* result.
*/
private class AppendFunction extends TaintTracking::FunctionModel {
AppendFunction() { this = Builtin::append() }

override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
inp.isParameter(_) and outp.isResult()
}
}

/**
* A model of the built-in `copy` function, which propagates taint from its second argument
* to its first.
*/
private class CopyFunction extends TaintTracking::FunctionModel {
CopyFunction() { this = Builtin::copy() }

override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
inp.isParameter(1) and outp.isParameter(0)
}
}

/**
* A model of the built-in `min` function, which computes the smallest value of a fixed number of
* arguments of ordered types. There is at least one argument and "ordered types" includes e.g.
* strings, so we care about data flow through `min`.
*/
private class MinFunction extends DataFlow::FunctionModel {
MinFunction() { this = Builtin::min_() }

override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp) {
inp.isParameter(_) and outp.isResult()
}
}

/**
* A model of the built-in `max` function, which computes the largest value of a fixed number of
* arguments of ordered types. There is at least one argument and "ordered types" includes e.g.
* strings, so we care about data flow through `max`.
*/
private class MaxFunction extends DataFlow::FunctionModel {
MaxFunction() { this = Builtin::max_() }

override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp) {
inp.isParameter(_) and outp.isResult()
}
}

/** Provides a class for modeling functions which convert strings into integers. */
module IntegerParser {
/**
Expand Down
28 changes: 14 additions & 14 deletions go/ql/test/experimental/CWE-090/LDAPInjection.expected
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
edges
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:671 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:59:3:59:11 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:61:3:61:51 | ...+... | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:3:62:33 | slice literal | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:62:24:62:32 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:66:3:66:11 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:68:3:68:51 | ...+... | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:3:69:33 | slice literal | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:69:24:69:32 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:73:3:73:11 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:75:3:75:51 | ...+... | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:3:76:33 | slice literal | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:76:24:76:32 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:80:22:80:30 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:57:15:57:29 | call to UserAgent | LDAPInjection.go:81:25:81:33 | untrusted | provenance | Src:MaD:674 |
| LDAPInjection.go:62:3:62:33 | slice literal [array] | LDAPInjection.go:62:3:62:33 | slice literal | provenance | |
| LDAPInjection.go:62:24:62:32 | untrusted | LDAPInjection.go:62:3:62:33 | slice literal [array] | provenance | |
| LDAPInjection.go:69:3:69:33 | slice literal [array] | LDAPInjection.go:69:3:69:33 | slice literal | provenance | |
Expand Down
6 changes: 3 additions & 3 deletions go/ql/test/experimental/CWE-203/Timing.expected
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
edges
| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | MaD:652 |
| timing.go:15:18:15:27 | selection of Header | timing.go:15:18:15:45 | call to Get | provenance | MaD:655 |
| timing.go:15:18:15:45 | call to Get | timing.go:17:31:17:42 | headerSecret | provenance | |
| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | MaD:652 |
| timing.go:28:18:28:27 | selection of Header | timing.go:28:18:28:45 | call to Get | provenance | MaD:655 |
| timing.go:28:18:28:45 | call to Get | timing.go:30:47:30:58 | headerSecret | provenance | |
| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | MaD:652 |
| timing.go:41:18:41:27 | selection of Header | timing.go:41:18:41:45 | call to Get | provenance | MaD:655 |
| timing.go:41:18:41:45 | call to Get | timing.go:42:25:42:36 | headerSecret | provenance | |
nodes
| timing.go:15:18:15:27 | selection of Header | semmle.label | selection of Header |
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
edges
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | MaD:732 |
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query | provenance | MaD:735 |
| ImproperLdapAuth.go:18:18:18:32 | call to Query | ImproperLdapAuth.go:28:23:28:34 | bindPassword | provenance | |
| ImproperLdapAuth.go:87:18:87:19 | "" | ImproperLdapAuth.go:97:23:97:34 | bindPassword | provenance | |
nodes
Expand Down
12 changes: 6 additions & 6 deletions go/ql/test/experimental/CWE-369/DivideByZero.expected
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
edges
| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | MaD:732 |
| DivideByZero.go:10:12:10:16 | selection of URL | DivideByZero.go:10:12:10:24 | call to Query | provenance | MaD:735 |
| DivideByZero.go:10:12:10:24 | call to Query | DivideByZero.go:11:27:11:32 | param1 | provenance | |
| DivideByZero.go:11:2:11:33 | ... := ...[0] | DivideByZero.go:12:16:12:20 | value | provenance | |
| DivideByZero.go:11:27:11:32 | param1 | DivideByZero.go:11:2:11:33 | ... := ...[0] | provenance | |
| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | MaD:732 |
| DivideByZero.go:17:12:17:16 | selection of URL | DivideByZero.go:17:12:17:24 | call to Query | provenance | MaD:735 |
| DivideByZero.go:17:12:17:24 | call to Query | DivideByZero.go:18:11:18:24 | type conversion | provenance | |
| DivideByZero.go:18:11:18:24 | type conversion | DivideByZero.go:19:16:19:20 | value | provenance | |
| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | MaD:732 |
| DivideByZero.go:24:12:24:16 | selection of URL | DivideByZero.go:24:12:24:24 | call to Query | provenance | MaD:735 |
| DivideByZero.go:24:12:24:24 | call to Query | DivideByZero.go:25:31:25:36 | param1 | provenance | |
| DivideByZero.go:25:2:25:45 | ... := ...[0] | DivideByZero.go:26:16:26:20 | value | provenance | |
| DivideByZero.go:25:31:25:36 | param1 | DivideByZero.go:25:2:25:45 | ... := ...[0] | provenance | |
| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | MaD:732 |
| DivideByZero.go:31:12:31:16 | selection of URL | DivideByZero.go:31:12:31:24 | call to Query | provenance | MaD:735 |
| DivideByZero.go:31:12:31:24 | call to Query | DivideByZero.go:32:33:32:38 | param1 | provenance | |
| DivideByZero.go:32:2:32:43 | ... := ...[0] | DivideByZero.go:33:16:33:20 | value | provenance | |
| DivideByZero.go:32:33:32:38 | param1 | DivideByZero.go:32:2:32:43 | ... := ...[0] | provenance | |
| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | MaD:732 |
| DivideByZero.go:38:12:38:16 | selection of URL | DivideByZero.go:38:12:38:24 | call to Query | provenance | MaD:735 |
| DivideByZero.go:38:12:38:24 | call to Query | DivideByZero.go:39:32:39:37 | param1 | provenance | |
| DivideByZero.go:39:2:39:46 | ... := ...[0] | DivideByZero.go:40:16:40:20 | value | provenance | |
| DivideByZero.go:39:32:39:37 | param1 | DivideByZero.go:39:2:39:46 | ... := ...[0] | provenance | |
| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | MaD:732 |
| DivideByZero.go:54:12:54:16 | selection of URL | DivideByZero.go:54:12:54:24 | call to Query | provenance | MaD:735 |
| DivideByZero.go:54:12:54:24 | call to Query | DivideByZero.go:55:11:55:24 | type conversion | provenance | |
| DivideByZero.go:55:11:55:24 | type conversion | DivideByZero.go:57:17:57:21 | value | provenance | |
nodes
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
edges
| test.go:59:16:59:44 | call to FormValue | test.go:128:20:128:27 | definition of filename | provenance | Src:MaD:667 |
| test.go:59:16:59:44 | call to FormValue | test.go:128:20:128:27 | definition of filename | provenance | Src:MaD:670 |
| test.go:60:15:60:26 | selection of Body | test.go:158:19:158:22 | definition of file | provenance | |
| test.go:61:24:61:35 | selection of Body | test.go:169:28:169:31 | definition of file | provenance | |
| test.go:62:13:62:24 | selection of Body | test.go:181:17:181:20 | definition of file | provenance | |
Expand Down Expand Up @@ -34,18 +34,18 @@ edges
| test.go:145:12:145:19 | call to Open | test.go:147:37:147:38 | rc | provenance | |
| test.go:158:19:158:22 | definition of file | test.go:159:25:159:28 | file | provenance | |
| test.go:159:2:159:29 | ... := ...[0] | test.go:160:48:160:52 | file1 | provenance | |
| test.go:159:25:159:28 | file | test.go:159:2:159:29 | ... := ...[0] | provenance | MaD:544 |
| test.go:159:25:159:28 | file | test.go:159:2:159:29 | ... := ...[0] | provenance | MaD:547 |
| test.go:160:2:160:69 | ... := ...[0] | test.go:163:26:163:29 | file | provenance | |
| test.go:160:32:160:53 | call to NewReader | test.go:160:2:160:69 | ... := ...[0] | provenance | |
| test.go:160:48:160:52 | file1 | test.go:160:32:160:53 | call to NewReader | provenance | MaD:40 |
| test.go:160:48:160:52 | file1 | test.go:160:32:160:53 | call to NewReader | provenance | MaD:43 |
| test.go:163:3:163:36 | ... := ...[0] | test.go:164:36:164:51 | fileReaderCloser | provenance | |
| test.go:163:26:163:29 | file | test.go:163:3:163:36 | ... := ...[0] | provenance | MaD:8 |
| test.go:169:28:169:31 | definition of file | test.go:170:25:170:28 | file | provenance | |
| test.go:170:2:170:29 | ... := ...[0] | test.go:171:57:171:61 | file2 | provenance | |
| test.go:170:25:170:28 | file | test.go:170:2:170:29 | ... := ...[0] | provenance | MaD:544 |
| test.go:170:25:170:28 | file | test.go:170:2:170:29 | ... := ...[0] | provenance | MaD:547 |
| test.go:171:2:171:78 | ... := ...[0] | test.go:175:26:175:29 | file | provenance | |
| test.go:171:41:171:62 | call to NewReader | test.go:171:2:171:78 | ... := ...[0] | provenance | |
| test.go:171:57:171:61 | file2 | test.go:171:41:171:62 | call to NewReader | provenance | MaD:40 |
| test.go:171:57:171:61 | file2 | test.go:171:41:171:62 | call to NewReader | provenance | MaD:43 |
| test.go:175:26:175:29 | file | test.go:175:26:175:36 | call to Open | provenance | |
| test.go:175:26:175:36 | call to Open | test.go:176:36:176:51 | fileReaderCloser | provenance | |
| test.go:181:17:181:20 | definition of file | test.go:184:41:184:44 | file | provenance | |
Expand Down
4 changes: 2 additions & 2 deletions go/ql/test/experimental/CWE-74/DsnInjection.expected
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
edges
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name | provenance | Src:MaD:667 |
| Dsn.go:49:11:49:106 | []type{args} [array] | Dsn.go:49:11:49:106 | call to Sprintf | provenance | MaD:242 |
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name | provenance | Src:MaD:670 |
| Dsn.go:49:11:49:106 | []type{args} [array] | Dsn.go:49:11:49:106 | call to Sprintf | provenance | MaD:245 |
| Dsn.go:49:11:49:106 | call to Sprintf | Dsn.go:50:29:50:33 | dbDSN | provenance | |
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | []type{args} [array] | provenance | |
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | call to Sprintf | provenance | FunctionModel |
Expand Down
4 changes: 2 additions & 2 deletions go/ql/test/experimental/CWE-74/DsnInjectionLocal.expected
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
edges
| Dsn.go:26:11:26:17 | selection of Args | Dsn.go:28:102:28:109 | index expression | provenance | |
| Dsn.go:28:11:28:110 | []type{args} [array] | Dsn.go:28:11:28:110 | call to Sprintf | provenance | MaD:242 |
| Dsn.go:28:11:28:110 | []type{args} [array] | Dsn.go:28:11:28:110 | call to Sprintf | provenance | MaD:245 |
| Dsn.go:28:11:28:110 | call to Sprintf | Dsn.go:29:29:29:33 | dbDSN | provenance | |
| Dsn.go:28:102:28:109 | index expression | Dsn.go:28:11:28:110 | []type{args} [array] | provenance | |
| Dsn.go:28:102:28:109 | index expression | Dsn.go:28:11:28:110 | call to Sprintf | provenance | FunctionModel |
Expand All @@ -12,7 +12,7 @@ edges
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:67:102:67:108 | selection of dsn | provenance | |
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:63:19:63:29 | slice expression | provenance | |
| Dsn.go:63:19:63:29 | slice expression | Dsn.go:63:9:63:11 | implicit dereference | provenance | FunctionModel |
| Dsn.go:67:11:67:109 | []type{args} [array] | Dsn.go:67:11:67:109 | call to Sprintf | provenance | MaD:242 |
| Dsn.go:67:11:67:109 | []type{args} [array] | Dsn.go:67:11:67:109 | call to Sprintf | provenance | MaD:245 |
| Dsn.go:67:11:67:109 | call to Sprintf | Dsn.go:68:29:68:33 | dbDSN | provenance | |
| Dsn.go:67:102:67:104 | cfg [pointer] | Dsn.go:67:102:67:104 | implicit dereference | provenance | |
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference | provenance | |
Expand Down