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.

Sign up for GitHub

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: Migrate AppenderOrSprinter model to models-as-data #15267

Merged
merged 4 commits into from
Jan 10, 2024
Merged

Go: Migrate AppenderOrSprinter model to models-as-data #15267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da4049e
Go: Migrate AppenderOrSprinter model to models-as-data
atorralba Jan 9, 2024
a0f6b5e
Update test expectations
atorralba Jan 9, 2024
dc911c3
Apply suggestions from code review
atorralba Jan 10, 2024
46df585
Update test expectations
atorralba Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions go/ql/lib/change-notes/2024-01-09-fmt-apprender-or-sprinter-deprecated.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: deprecated
---
* The class `Fmt::AppenderOrSprinter` of the `Fmt.qll` module has been deprecated. Use the new `Fmt::AppenderOrSprinterFunc` class instead. Its taint flow features have been migrated to models-as-data.
11 changes: 11 additions & 0 deletions go/ql/lib/ext/fmt.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,14 @@ extensions:
- ["fmt", "ScanState", True, "Token", "", "", "Argument[-1]", "ReturnValue[0]", "taint", "manual"]
- ["fmt", "State", True, "Write", "", "", "Argument[0]", "Argument[-1]", "taint", "manual"]
- ["fmt", "Stringer", True, "String", "", "", "Argument[-1]", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Append", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Append", "", "", "Argument[1].ArrayElement", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Appendf", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Appendf", "", "", "Argument[1]", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Appendf", "", "", "Argument[2].ArrayElement", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Appendln", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Appendln", "", "", "Argument[1].ArrayElement", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Sprint", "", "", "Argument[0].ArrayElement", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Sprintf", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Sprintf", "", "", "Argument[1].ArrayElement", "ReturnValue", "taint", "manual"]
- ["fmt", "", True, "Sprintln", "", "", "Argument[0].ArrayElement", "ReturnValue", "taint", "manual"]
17 changes: 14 additions & 3 deletions go/ql/lib/semmle/go/frameworks/stdlib/Fmt.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,28 @@ import go
// Some TaintTracking::FunctionModel subclasses remain because varargs functions don't work with Models-as-Data sumamries yet.
/** Provides models of commonly used functions in the `fmt` package. */
module Fmt {
/** The `Sprint` or `Append` functions or one of their variants. */
class AppenderOrSprinter extends TaintTracking::FunctionModel {
/**
* The `Sprint` or `Append` functions or one of their variants.
*
* DEPRECATED: Use AppenderOrSprinterFunc instead.
*/
deprecated class AppenderOrSprinter extends TaintTracking::FunctionModel {
AppenderOrSprinter() { this.hasQualifiedName("fmt", ["Append", "Sprint"] + ["", "f", "ln"]) }

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

/** The `Sprint` or `Append` functions or one of their variants. */
class AppenderOrSprinterFunc extends Function {
AppenderOrSprinterFunc() {
this.hasQualifiedName("fmt", ["Append", "Sprint"] + ["", "f", "ln"])
}
}

/** The `Sprint` function or one of its variants. */
class Sprinter extends AppenderOrSprinter {
class Sprinter extends AppenderOrSprinterFunc {
Sprinter() { this.getName().matches("Sprint%") }
}

Expand Down
2 changes: 1 addition & 1 deletion go/ql/src/Security/CWE-352/ConstantOauth2State.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ module PrivateUrlFlowsToAuthCodeUrlCallConfig implements DataFlow::ConfigSig {
or
// Propagate across Sprintf and similar calls
exists(DataFlow::CallNode cn |
cn.getACalleeIncludingExternals().asFunction() instanceof Fmt::AppenderOrSprinter
cn.getACalleeIncludingExternals().asFunction() instanceof Fmt::AppenderOrSprinterFunc
|
pred = cn.getASyntacticArgument() and succ = cn.getResult()
)
Expand Down
3 changes: 3 additions & 0 deletions go/ql/test/experimental/CWE-74/DsnInjection.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
edges
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name |
| Dsn.go:49:11:49:106 | []type{args} [array] | Dsn.go:49:11:49:106 | call to Sprintf |
| Dsn.go:49:11:49:106 | call to Sprintf | Dsn.go:50:29:50:33 | dbDSN |
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | []type{args} [array] |
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | call to Sprintf |
nodes
| Dsn.go:47:10:47:30 | call to FormValue | semmle.label | call to FormValue |
| Dsn.go:49:11:49:106 | []type{args} [array] | semmle.label | []type{args} [array] |
| Dsn.go:49:11:49:106 | call to Sprintf | semmle.label | call to Sprintf |
| Dsn.go:49:102:49:105 | name | semmle.label | name |
| Dsn.go:50:29:50:33 | dbDSN | semmle.label | dbDSN |
Expand Down
6 changes: 6 additions & 0 deletions go/ql/test/experimental/CWE-74/DsnInjectionLocal.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
edges
| Dsn.go:26:11:26:17 | selection of Args | Dsn.go:28:102:28:109 | index expression |
| Dsn.go:28:11:28:110 | []type{args} [array] | Dsn.go:28:11:28:110 | call to Sprintf |
| Dsn.go:28:11:28:110 | call to Sprintf | Dsn.go:29:29:29:33 | dbDSN |
| Dsn.go:28:102:28:109 | index expression | Dsn.go:28:11:28:110 | []type{args} [array] |
| Dsn.go:28:102:28:109 | index expression | Dsn.go:28:11:28:110 | call to Sprintf |
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:63:9:63:11 | cfg [pointer] |
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:67:102:67:104 | cfg [pointer] |
Expand All @@ -10,13 +12,16 @@ edges
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:67:102:67:108 | selection of dsn |
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:63:19:63:29 | slice expression |
| Dsn.go:63:19:63:29 | slice expression | Dsn.go:63:9:63:11 | implicit dereference |
| Dsn.go:67:11:67:109 | []type{args} [array] | Dsn.go:67:11:67:109 | call to Sprintf |
| Dsn.go:67:11:67:109 | call to Sprintf | Dsn.go:68:29:68:33 | dbDSN |
| Dsn.go:67:102:67:104 | cfg [pointer] | Dsn.go:67:102:67:104 | implicit dereference |
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:67:102:67:108 | selection of dsn |
| Dsn.go:67:102:67:108 | selection of dsn | Dsn.go:67:11:67:109 | []type{args} [array] |
| Dsn.go:67:102:67:108 | selection of dsn | Dsn.go:67:11:67:109 | call to Sprintf |
nodes
| Dsn.go:26:11:26:17 | selection of Args | semmle.label | selection of Args |
| Dsn.go:28:11:28:110 | []type{args} [array] | semmle.label | []type{args} [array] |
| Dsn.go:28:11:28:110 | call to Sprintf | semmle.label | call to Sprintf |
| Dsn.go:28:102:28:109 | index expression | semmle.label | index expression |
| Dsn.go:29:29:29:33 | dbDSN | semmle.label | dbDSN |
Expand All @@ -25,6 +30,7 @@ nodes
| Dsn.go:63:9:63:11 | implicit dereference | semmle.label | implicit dereference |
| Dsn.go:63:19:63:25 | selection of Args | semmle.label | selection of Args |
| Dsn.go:63:19:63:29 | slice expression | semmle.label | slice expression |
| Dsn.go:67:11:67:109 | []type{args} [array] | semmle.label | []type{args} [array] |
| Dsn.go:67:11:67:109 | call to Sprintf | semmle.label | call to Sprintf |
| Dsn.go:67:102:67:104 | cfg [pointer] | semmle.label | cfg [pointer] |
| Dsn.go:67:102:67:104 | implicit dereference | semmle.label | implicit dereference |
Expand Down
18 changes: 18 additions & 0 deletions go/ql/test/experimental/CWE-918/SSRF.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ edges
| new-tests.go:26:26:26:30 | &... | new-tests.go:31:48:31:56 | selection of word |
| new-tests.go:26:26:26:30 | &... | new-tests.go:32:48:32:56 | selection of safe |
| new-tests.go:26:26:26:30 | &... | new-tests.go:35:49:35:57 | selection of word |
| new-tests.go:31:11:31:57 | []type{args} [array] | new-tests.go:31:11:31:57 | call to Sprintf |
| new-tests.go:31:48:31:56 | selection of word | new-tests.go:31:11:31:57 | []type{args} [array] |
| new-tests.go:31:48:31:56 | selection of word | new-tests.go:31:11:31:57 | call to Sprintf |
| new-tests.go:32:11:32:57 | []type{args} [array] | new-tests.go:32:11:32:57 | call to Sprintf |
| new-tests.go:32:48:32:56 | selection of safe | new-tests.go:32:11:32:57 | []type{args} [array] |
| new-tests.go:32:48:32:56 | selection of safe | new-tests.go:32:11:32:57 | call to Sprintf |
| new-tests.go:35:12:35:58 | []type{args} [array] | new-tests.go:35:12:35:58 | call to Sprintf |
| new-tests.go:35:49:35:57 | selection of word | new-tests.go:35:12:35:58 | []type{args} [array] |
| new-tests.go:35:49:35:57 | selection of word | new-tests.go:35:12:35:58 | call to Sprintf |
| new-tests.go:39:18:39:30 | call to Param | new-tests.go:47:11:47:46 | ...+... |
| new-tests.go:49:18:49:30 | call to Query | new-tests.go:50:11:50:46 | ...+... |
Expand All @@ -18,8 +24,14 @@ edges
| new-tests.go:63:26:63:30 | &... | new-tests.go:68:48:68:56 | selection of word |
| new-tests.go:63:26:63:30 | &... | new-tests.go:69:48:69:56 | selection of safe |
| new-tests.go:63:26:63:30 | &... | new-tests.go:74:49:74:57 | selection of word |
| new-tests.go:68:11:68:57 | []type{args} [array] | new-tests.go:68:11:68:57 | call to Sprintf |
| new-tests.go:68:48:68:56 | selection of word | new-tests.go:68:11:68:57 | []type{args} [array] |
| new-tests.go:68:48:68:56 | selection of word | new-tests.go:68:11:68:57 | call to Sprintf |
| new-tests.go:69:11:69:57 | []type{args} [array] | new-tests.go:69:11:69:57 | call to Sprintf |
| new-tests.go:69:48:69:56 | selection of safe | new-tests.go:69:11:69:57 | []type{args} [array] |
| new-tests.go:69:48:69:56 | selection of safe | new-tests.go:69:11:69:57 | call to Sprintf |
| new-tests.go:74:12:74:58 | []type{args} [array] | new-tests.go:74:12:74:58 | call to Sprintf |
| new-tests.go:74:49:74:57 | selection of word | new-tests.go:74:12:74:58 | []type{args} [array] |
| new-tests.go:74:49:74:57 | selection of word | new-tests.go:74:12:74:58 | call to Sprintf |
| new-tests.go:78:18:78:24 | selection of URL | new-tests.go:78:18:78:32 | call to Query |
| new-tests.go:78:18:78:32 | call to Query | new-tests.go:78:18:78:46 | call to Get |
Expand All @@ -41,10 +53,13 @@ nodes
| builtin.go:129:21:129:31 | call to Referer | semmle.label | call to Referer |
| builtin.go:132:38:132:51 | untrustedInput | semmle.label | untrustedInput |
| new-tests.go:26:26:26:30 | &... | semmle.label | &... |
| new-tests.go:31:11:31:57 | []type{args} [array] | semmle.label | []type{args} [array] |
| new-tests.go:31:11:31:57 | call to Sprintf | semmle.label | call to Sprintf |
| new-tests.go:31:48:31:56 | selection of word | semmle.label | selection of word |
| new-tests.go:32:11:32:57 | []type{args} [array] | semmle.label | []type{args} [array] |
| new-tests.go:32:11:32:57 | call to Sprintf | semmle.label | call to Sprintf |
| new-tests.go:32:48:32:56 | selection of safe | semmle.label | selection of safe |
| new-tests.go:35:12:35:58 | []type{args} [array] | semmle.label | []type{args} [array] |
| new-tests.go:35:12:35:58 | call to Sprintf | semmle.label | call to Sprintf |
| new-tests.go:35:49:35:57 | selection of word | semmle.label | selection of word |
| new-tests.go:39:18:39:30 | call to Param | semmle.label | call to Param |
Expand All @@ -55,10 +70,13 @@ nodes
| new-tests.go:62:31:62:38 | selection of Body | semmle.label | selection of Body |
| new-tests.go:63:17:63:23 | reqBody | semmle.label | reqBody |
| new-tests.go:63:26:63:30 | &... | semmle.label | &... |
| new-tests.go:68:11:68:57 | []type{args} [array] | semmle.label | []type{args} [array] |
| new-tests.go:68:11:68:57 | call to Sprintf | semmle.label | call to Sprintf |
| new-tests.go:68:48:68:56 | selection of word | semmle.label | selection of word |
| new-tests.go:69:11:69:57 | []type{args} [array] | semmle.label | []type{args} [array] |
| new-tests.go:69:11:69:57 | call to Sprintf | semmle.label | call to Sprintf |
| new-tests.go:69:48:69:56 | selection of safe | semmle.label | selection of safe |
| new-tests.go:74:12:74:58 | []type{args} [array] | semmle.label | []type{args} [array] |
| new-tests.go:74:12:74:58 | call to Sprintf | semmle.label | call to Sprintf |
| new-tests.go:74:49:74:57 | selection of word | semmle.label | selection of word |
| new-tests.go:78:18:78:24 | selection of URL | semmle.label | selection of URL |
Expand Down
11 changes: 11 additions & 0 deletions go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
| file://:0:0:0:0 | [summary param] 0 in Append | file://:0:0:0:0 | [summary] to write: ReturnValue in Append |
| file://:0:0:0:0 | [summary param] 0 in AppendQuote | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendQuote |
| file://:0:0:0:0 | [summary param] 0 in AppendQuoteToASCII | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendQuoteToASCII |
| file://:0:0:0:0 | [summary param] 0 in AppendQuoteToGraphic | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendQuoteToGraphic |
| file://:0:0:0:0 | [summary param] 0 in AppendSlice | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendSlice |
| file://:0:0:0:0 | [summary param] 0 in Appendf | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendf |
| file://:0:0:0:0 | [summary param] 0 in Appendln | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendln |
| file://:0:0:0:0 | [summary param] 0 in As | file://:0:0:0:0 | [summary] to write: Argument[1] in As |
| file://:0:0:0:0 | [summary param] 0 in Base | file://:0:0:0:0 | [summary] to write: ReturnValue in Base |
| file://:0:0:0:0 | [summary param] 0 in BytePtrFromString | file://:0:0:0:0 | [summary] to write: ReturnValue in BytePtrFromString |
Expand Down Expand Up @@ -67,6 +70,7 @@
| file://:0:0:0:0 | [summary param] 0 in SplitAfter | file://:0:0:0:0 | [summary] to write: ReturnValue in SplitAfter |
| file://:0:0:0:0 | [summary param] 0 in SplitAfterN | file://:0:0:0:0 | [summary] to write: ReturnValue in SplitAfterN |
| file://:0:0:0:0 | [summary param] 0 in SplitN | file://:0:0:0:0 | [summary] to write: ReturnValue in SplitN |
| file://:0:0:0:0 | [summary param] 0 in Sprintf | file://:0:0:0:0 | [summary] to write: ReturnValue in Sprintf |
| file://:0:0:0:0 | [summary param] 0 in Store | file://:0:0:0:0 | [summary] to write: Argument[-1] in Store |
| file://:0:0:0:0 | [summary param] 0 in Store | file://:0:0:0:0 | [summary] to write: Argument[-1] in Store |
| file://:0:0:0:0 | [summary param] 0 in Store | file://:0:0:0:0 | [summary] to write: Argument[-1] in Store |
Expand Down Expand Up @@ -133,6 +137,7 @@
| file://:0:0:0:0 | [summary param] 1 in AppendQuoteToASCII | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendQuoteToASCII |
| file://:0:0:0:0 | [summary param] 1 in AppendQuoteToGraphic | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendQuoteToGraphic |
| file://:0:0:0:0 | [summary param] 1 in AppendSlice | file://:0:0:0:0 | [summary] to write: ReturnValue in AppendSlice |
| file://:0:0:0:0 | [summary param] 1 in Appendf | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendf |
| file://:0:0:0:0 | [summary param] 1 in Copy | file://:0:0:0:0 | [summary] to write: Argument[0] in Copy |
| file://:0:0:0:0 | [summary param] 1 in Copy | file://:0:0:0:0 | [summary] to write: Argument[0] in Copy |
| file://:0:0:0:0 | [summary param] 1 in CopyBuffer | file://:0:0:0:0 | [summary] to write: Argument[0] in CopyBuffer |
Expand Down Expand Up @@ -287,6 +292,12 @@
| file://:0:0:0:0 | [summary param] -1 in WriteTo | file://:0:0:0:0 | [summary] to write: Argument[0] in WriteTo |
| file://:0:0:0:0 | [summary param] -1 in WriteTo | file://:0:0:0:0 | [summary] to write: Argument[0] in WriteTo |
| file://:0:0:0:0 | [summary param] -1 in WriteTo | file://:0:0:0:0 | [summary] to write: Argument[0] in WriteTo |
| file://:0:0:0:0 | [summary] read: Argument[0].ArrayElement in Sprint | file://:0:0:0:0 | [summary] to write: ReturnValue in Sprint |
| file://:0:0:0:0 | [summary] read: Argument[0].ArrayElement in Sprintln | file://:0:0:0:0 | [summary] to write: ReturnValue in Sprintln |
| file://:0:0:0:0 | [summary] read: Argument[1].ArrayElement in Append | file://:0:0:0:0 | [summary] to write: ReturnValue in Append |
| file://:0:0:0:0 | [summary] read: Argument[1].ArrayElement in Appendln | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendln |
| file://:0:0:0:0 | [summary] read: Argument[1].ArrayElement in Sprintf | file://:0:0:0:0 | [summary] to write: ReturnValue in Sprintf |
| file://:0:0:0:0 | [summary] read: Argument[2].ArrayElement in Appendf | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendf |
| main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[0] |
| main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[1] |
| main.go:38:13:38:13 | 1 | main.go:38:7:38:20 | slice literal |
Expand Down
5 changes: 5 additions & 0 deletions go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/TaintStep.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,14 @@
| errors.As | file://:0:0:0:0 | [summary param] 0 in As | file://:0:0:0:0 | [summary] to write: Argument[1] in As |
| errors.New | file://:0:0:0:0 | [summary param] 0 in New | file://:0:0:0:0 | [summary] to write: ReturnValue in New |
| errors.Unwrap | file://:0:0:0:0 | [summary param] 0 in Unwrap | file://:0:0:0:0 | [summary] to write: ReturnValue in Unwrap |
| fmt.Append | file://:0:0:0:0 | [summary param] 0 in Append | file://:0:0:0:0 | [summary] to write: ReturnValue in Append |
| fmt.Appendf | file://:0:0:0:0 | [summary param] 0 in Appendf | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendf |
| fmt.Appendf | file://:0:0:0:0 | [summary param] 1 in Appendf | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendf |
| fmt.Appendln | file://:0:0:0:0 | [summary param] 0 in Appendln | file://:0:0:0:0 | [summary] to write: ReturnValue in Appendln |
| fmt.GoStringer.GoString | file://:0:0:0:0 | [summary param] -1 in GoString | file://:0:0:0:0 | [summary] to write: ReturnValue in GoString |
| fmt.ScanState.Read | file://:0:0:0:0 | [summary param] -1 in Read | file://:0:0:0:0 | [summary] to write: Argument[0] in Read |
| fmt.ScanState.Token | file://:0:0:0:0 | [summary param] -1 in Token | file://:0:0:0:0 | [summary] to write: ReturnValue in Token |
| fmt.Sprintf | file://:0:0:0:0 | [summary param] 0 in Sprintf | file://:0:0:0:0 | [summary] to write: ReturnValue in Sprintf |
| fmt.State.Write | file://:0:0:0:0 | [summary param] 0 in Write | file://:0:0:0:0 | [summary] to write: Argument[-1] in Write |
| fmt.Stringer.String | file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
| fmt.pp.Write | file://:0:0:0:0 | [summary param] 0 in Write | file://:0:0:0:0 | [summary] to write: Argument[-1] in Write |
Expand Down
Loading
Loading