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

Revert "Go: Make models-as-data source models for variadic parameters work" #18433

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Revert "Go: Make models-as-data source models for variadic parameters work" #18433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 4 deletions go/ql/lib/change-notes/2024-12-12-variadic-parameter-sources.md
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,3 @@ class ContentApprox = Unit;
/** Gets an approximated value for content `c`. */
pragma[inline]
ContentApprox getContentApprox(Content c) { any() }

/**
* Holds if the the content `c` is a container.
*/
predicate containerContent(ContentSet c) {
c instanceof ArrayContent or
c instanceof CollectionContent or
c instanceof MapKeyContent or
c instanceof MapValueContent
}
38 changes: 8 additions & 30 deletions go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,11 @@ predicate localExprTaint(Expr src, Expr sink) {
* Holds if taint can flow in one local step from `src` to `sink`.
*/
predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
DataFlow::localFlowStep(src, sink)
or
localAdditionalTaintStep(src, sink, _)
or
DataFlow::localFlowStep(src, sink) or
localAdditionalTaintStep(src, sink, _) or
// Simple flow through library code is included in the exposed local
// step relation, even though flow is technically inter-procedural
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(src, sink, _)
or
// Treat container flow as taint for the local taint flow relation
exists(DataFlow::Content c | DataFlowPrivate::containerContent(c) |
DataFlowPrivate::readStep(src, c, sink) or
DataFlowPrivate::storeStep(src, c, sink) or
FlowSummaryImpl::Private::Steps::summaryGetterStep(src, c, sink, _) or
FlowSummaryImpl::Private::Steps::summarySetterStep(src, c, sink, _)
)
}

private Type getElementType(Type containerType) {
Expand Down Expand Up @@ -98,18 +88,12 @@ class AdditionalTaintStep extends Unit {
*/
predicate localAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ, string model) {
(
referenceStep(pred, succ)
or
elementWriteStep(pred, succ)
or
fieldReadStep(pred, succ)
or
elementStep(pred, succ)
or
tupleStep(pred, succ)
or
stringConcatStep(pred, succ)
or
referenceStep(pred, succ) or
elementWriteStep(pred, succ) or
fieldReadStep(pred, succ) or
elementStep(pred, succ) or
tupleStep(pred, succ) or
stringConcatStep(pred, succ) or
sliceStep(pred, succ)
) and
model = ""
Expand Down Expand Up @@ -179,12 +163,6 @@ predicate elementStep(DataFlow::Node pred, DataFlow::Node succ) {
// only step into the value, not the index
succ.asInstruction() = IR::extractTupleElement(nextEntry, 1)
)
or
exists(DataFlow::ImplicitVarargsSlice ivs |
pred.(DataFlow::PostUpdateNode).getPreUpdateNode() = ivs and
succ.(DataFlow::PostUpdateNode).getPreUpdateNode() =
ivs.getCallNode().getAnImplicitVarargsArgument()
)
}

/** Holds if taint flows from `pred` to `succ` via an extract tuple operation. */
Expand Down
28 changes: 0 additions & 28 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
Expand Up @@ -5,36 +5,24 @@
| main.go:38:19:38:19 | 3 | main.go:38:7:38:20 | slice literal |
| main.go:39:8:39:25 | []type{args} | main.go:39:8:39:25 | call to append |
| main.go:39:15:39:15 | s | main.go:39:8:39:25 | call to append |
| main.go:39:18:39:18 | 4 | main.go:39:8:39:25 | []type{args} |
| main.go:39:21:39:21 | 5 | main.go:39:8:39:25 | []type{args} |
| main.go:39:24:39:24 | 6 | main.go:39:8:39:25 | []type{args} |
| main.go:40:15:40:15 | s | main.go:40:8:40:23 | call to append |
| main.go:40:18:40:19 | s1 | main.go:40:8:40:23 | call to append |
| main.go:42:10:42:11 | s4 | main.go:38:2:38:2 | definition of s |
| main.go:47:20:47:21 | next key-value pair in range | main.go:47:2:50:2 | range statement[0] |
| main.go:47:20:47:21 | next key-value pair in range | main.go:47:2:50:2 | range statement[1] |
| main.go:47:20:47:21 | xs | main.go:47:2:50:2 | range statement[1] |
| main.go:56:8:56:11 | true | main.go:56:2:56:3 | ch |
| main.go:57:4:57:5 | ch | main.go:57:2:57:5 | <-... |
| strings.go:9:24:9:24 | s | strings.go:9:8:9:38 | call to Replace |
| strings.go:9:32:9:34 | "_" | strings.go:9:8:9:38 | call to Replace |
| strings.go:10:27:10:27 | s | strings.go:10:8:10:42 | call to ReplaceAll |
| strings.go:10:35:10:41 | "&amp;" | strings.go:10:8:10:42 | call to ReplaceAll |
| strings.go:11:9:11:26 | []type{args} | strings.go:11:9:11:26 | call to Sprint |
| strings.go:11:9:11:26 | call to Sprint | strings.go:11:9:11:50 | ...+... |
| strings.go:11:9:11:50 | ...+... | strings.go:11:9:11:69 | ...+... |
| strings.go:11:20:11:21 | s2 | strings.go:11:9:11:26 | []type{args} |
| strings.go:11:20:11:21 | s2 | strings.go:11:9:11:26 | call to Sprint |
| strings.go:11:24:11:25 | s3 | strings.go:11:9:11:26 | []type{args} |
| strings.go:11:24:11:25 | s3 | strings.go:11:9:11:26 | call to Sprint |
| strings.go:11:30:11:50 | []type{args} | strings.go:11:30:11:50 | call to Sprintf |
| strings.go:11:30:11:50 | call to Sprintf | strings.go:11:9:11:50 | ...+... |
| strings.go:11:42:11:45 | "%q" | strings.go:11:30:11:50 | call to Sprintf |
| strings.go:11:48:11:49 | s2 | strings.go:11:30:11:50 | []type{args} |
| strings.go:11:48:11:49 | s2 | strings.go:11:30:11:50 | call to Sprintf |
| strings.go:11:54:11:69 | []type{args} | strings.go:11:54:11:69 | call to Sprintln |
| strings.go:11:54:11:69 | call to Sprintln | strings.go:11:9:11:69 | ...+... |
| strings.go:11:67:11:68 | s3 | strings.go:11:54:11:69 | []type{args} |
| strings.go:11:67:11:68 | s3 | strings.go:11:54:11:69 | call to Sprintln |
| url.go:12:14:12:48 | call to PathUnescape | url.go:12:3:12:48 | ... = ...[0] |
| url.go:12:14:12:48 | call to PathUnescape | url.go:12:3:12:48 | ... = ...[1] |
Expand All @@ -51,25 +39,17 @@
| url.go:27:9:27:30 | call to ParseRequestURI | url.go:27:2:27:30 | ... = ...[1] |
| url.go:27:29:27:29 | s | url.go:27:2:27:30 | ... = ...[0] |
| url.go:28:14:28:14 | u | url.go:28:14:28:28 | call to EscapedPath |
| url.go:28:14:28:28 | call to EscapedPath | url.go:28:2:28:29 | []type{args} |
| url.go:29:14:29:14 | u | url.go:29:14:29:25 | call to Hostname |
| url.go:29:14:29:25 | call to Hostname | url.go:29:2:29:26 | []type{args} |
| url.go:30:11:30:11 | u | url.go:30:2:30:27 | ... := ...[0] |
| url.go:30:11:30:27 | call to MarshalBinary | url.go:30:2:30:27 | ... := ...[0] |
| url.go:30:11:30:27 | call to MarshalBinary | url.go:30:2:30:27 | ... := ...[1] |
| url.go:31:2:31:16 | []type{args} | url.go:30:2:30:3 | definition of bs |
| url.go:31:14:31:15 | bs | url.go:31:2:31:16 | []type{args} |
| url.go:32:9:32:9 | u | url.go:32:2:32:23 | ... = ...[0] |
| url.go:32:9:32:23 | call to Parse | url.go:32:2:32:23 | ... = ...[0] |
| url.go:32:9:32:23 | call to Parse | url.go:32:2:32:23 | ... = ...[1] |
| url.go:32:17:32:22 | "/foo" | url.go:32:2:32:23 | ... = ...[0] |
| url.go:33:14:33:14 | u | url.go:33:14:33:21 | call to Port |
| url.go:33:14:33:21 | call to Port | url.go:33:2:33:22 | []type{args} |
| url.go:34:2:34:23 | []type{args} | url.go:34:14:34:22 | call to Query |
| url.go:34:14:34:14 | u | url.go:34:14:34:22 | call to Query |
| url.go:34:14:34:22 | call to Query | url.go:34:2:34:23 | []type{args} |
| url.go:35:14:35:14 | u | url.go:35:14:35:27 | call to RequestURI |
| url.go:35:14:35:27 | call to RequestURI | url.go:35:2:35:28 | []type{args} |
| url.go:36:6:36:6 | u | url.go:36:6:36:26 | call to ResolveReference |
| url.go:36:25:36:25 | u | url.go:36:6:36:26 | call to ResolveReference |
| url.go:41:17:41:20 | "me" | url.go:41:8:41:21 | call to User |
Expand All @@ -78,35 +58,27 @@
| url.go:43:11:43:12 | ui | url.go:43:2:43:23 | ... := ...[0] |
| url.go:43:11:43:23 | call to Password | url.go:43:2:43:23 | ... := ...[0] |
| url.go:43:11:43:23 | call to Password | url.go:43:2:43:23 | ... := ...[1] |
| url.go:44:14:44:15 | pw | url.go:44:2:44:16 | []type{args} |
| url.go:45:14:45:15 | ui | url.go:45:14:45:26 | call to Username |
| url.go:45:14:45:26 | call to Username | url.go:45:2:45:27 | []type{args} |
| url.go:50:10:50:26 | call to ParseQuery | url.go:50:2:50:26 | ... := ...[0] |
| url.go:50:10:50:26 | call to ParseQuery | url.go:50:2:50:26 | ... := ...[1] |
| url.go:50:25:50:25 | q | url.go:50:2:50:26 | ... := ...[0] |
| url.go:51:14:51:14 | v | url.go:51:14:51:23 | call to Encode |
| url.go:51:14:51:23 | call to Encode | url.go:51:2:51:24 | []type{args} |
| url.go:52:14:52:14 | v | url.go:52:14:52:26 | call to Get |
| url.go:52:14:52:26 | call to Get | url.go:52:2:52:27 | []type{args} |
| url.go:57:16:57:39 | call to JoinPath | url.go:57:2:57:39 | ... := ...[0] |
| url.go:57:16:57:39 | call to JoinPath | url.go:57:2:57:39 | ... := ...[1] |
| url.go:57:29:57:29 | q | url.go:57:2:57:39 | ... := ...[0] |
| url.go:57:32:57:38 | "clean" | url.go:57:2:57:39 | ... := ...[0] |
| url.go:57:32:57:38 | "clean" | url.go:57:16:57:39 | []type{args} |
| url.go:58:16:58:45 | call to JoinPath | url.go:58:2:58:45 | ... := ...[0] |
| url.go:58:16:58:45 | call to JoinPath | url.go:58:2:58:45 | ... := ...[1] |
| url.go:58:29:58:35 | "clean" | url.go:58:2:58:45 | ... := ...[0] |
| url.go:58:38:58:44 | joined1 | url.go:58:2:58:45 | ... := ...[0] |
| url.go:58:38:58:44 | joined1 | url.go:58:16:58:45 | []type{args} |
| url.go:59:14:59:31 | call to Parse | url.go:59:2:59:31 | ... := ...[0] |
| url.go:59:14:59:31 | call to Parse | url.go:59:2:59:31 | ... := ...[1] |
| url.go:59:24:59:30 | joined2 | url.go:59:2:59:31 | ... := ...[0] |
| url.go:60:15:60:19 | asUrl | url.go:60:15:60:37 | call to JoinPath |
| url.go:60:30:60:36 | "clean" | url.go:60:15:60:37 | []type{args} |
| url.go:60:30:60:36 | "clean" | url.go:60:15:60:37 | call to JoinPath |
| url.go:65:17:65:48 | call to Parse | url.go:65:2:65:48 | ... := ...[0] |
| url.go:65:17:65:48 | call to Parse | url.go:65:2:65:48 | ... := ...[1] |
| url.go:65:27:65:47 | "http://harmless.org" | url.go:65:2:65:48 | ... := ...[0] |
| url.go:66:9:66:16 | cleanUrl | url.go:66:9:66:28 | call to JoinPath |
| url.go:66:27:66:27 | q | url.go:66:9:66:28 | []type{args} |
| url.go:66:27:66:27 | q | url.go:66:9:66:28 | call to JoinPath |
8 changes: 1 addition & 7 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,7 @@ func main() {

var variadicSource string
test.VariadicSource(&variadicSource)
sink(variadicSource) // $ hasTaintFlow="variadicSource"
sink(&variadicSource) // $ hasTaintFlow="&..."

var variadicSourcePtr *string
test.VariadicSource(variadicSourcePtr)
sink(variadicSourcePtr) // $ hasTaintFlow="variadicSourcePtr"
sink(*variadicSourcePtr) // $ hasTaintFlow="star expression"
sink(variadicSource) // $ MISSING: hasTaintFlow="variadicSource"
dbartol marked this conversation as resolved.
Show resolved Hide resolved

test.VariadicSink(source()) // $ hasTaintFlow="[]type{args}"

Expand Down
12 changes: 0 additions & 12 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 @@ -10,13 +10,9 @@ invalidModelRow
| io.go:14:31:14:43 | "some string" | io.go:14:13:14:44 | call to NewReader |
| io.go:16:3:16:3 | definition of w | io.go:16:23:16:27 | &... |
| io.go:16:3:16:3 | definition of w | io.go:16:30:16:34 | &... |
| io.go:16:8:16:35 | []type{args} | io.go:16:23:16:27 | &... |
| io.go:16:8:16:35 | []type{args} | io.go:16:30:16:34 | &... |
| io.go:16:23:16:27 | &... | io.go:15:7:15:10 | definition of buf1 |
| io.go:16:23:16:27 | &... | io.go:16:8:16:35 | []type{args} |
| io.go:16:24:16:27 | buf1 | io.go:16:23:16:27 | &... |
| io.go:16:30:16:34 | &... | io.go:15:13:15:16 | definition of buf2 |
| io.go:16:30:16:34 | &... | io.go:16:8:16:35 | []type{args} |
| io.go:16:31:16:34 | buf2 | io.go:16:30:16:34 | &... |
| io.go:18:14:18:19 | reader | io.go:16:3:16:3 | definition of w |
| io.go:22:31:22:43 | "some string" | io.go:22:13:22:44 | call to NewReader |
Expand All @@ -31,10 +27,8 @@ invalidModelRow
| io.go:39:11:39:19 | call to Pipe | io.go:39:3:39:19 | ... := ...[0] |
| io.go:39:11:39:19 | call to Pipe | io.go:39:3:39:19 | ... := ...[1] |
| io.go:40:17:40:31 | "some string\\n" | io.go:39:6:39:6 | definition of w |
| io.go:40:17:40:31 | "some string\\n" | io.go:40:3:40:32 | []type{args} |
| io.go:43:16:43:16 | r | io.go:42:3:42:5 | definition of buf |
| io.go:44:13:44:15 | buf | io.go:44:13:44:24 | call to String |
| io.go:44:13:44:24 | call to String | io.go:44:3:44:25 | []type{args} |
| io.go:48:31:48:43 | "some string" | io.go:48:13:48:44 | call to NewReader |
| io.go:50:18:50:23 | reader | io.go:49:3:49:5 | definition of buf |
| io.go:54:31:54:43 | "some string" | io.go:54:13:54:44 | call to NewReader |
Expand All @@ -52,14 +46,8 @@ invalidModelRow
| io.go:82:27:82:36 | "reader1 " | io.go:82:9:82:37 | call to NewReader |
| io.go:83:27:83:36 | "reader2 " | io.go:83:9:83:37 | call to NewReader |
| io.go:84:27:84:35 | "reader3" | io.go:84:9:84:36 | call to NewReader |
| io.go:85:8:85:33 | []type{args} | io.go:82:3:82:4 | definition of r1 |
| io.go:85:8:85:33 | []type{args} | io.go:83:3:83:4 | definition of r2 |
| io.go:85:8:85:33 | []type{args} | io.go:84:3:84:4 | definition of r3 |
| io.go:85:23:85:24 | r1 | io.go:85:8:85:33 | []type{args} |
| io.go:85:23:85:24 | r1 | io.go:85:8:85:33 | call to MultiReader |
| io.go:85:27:85:28 | r2 | io.go:85:8:85:33 | []type{args} |
| io.go:85:27:85:28 | r2 | io.go:85:8:85:33 | call to MultiReader |
| io.go:85:31:85:32 | r3 | io.go:85:8:85:33 | []type{args} |
| io.go:85:31:85:32 | r3 | io.go:85:8:85:33 | call to MultiReader |
| io.go:86:22:86:22 | r | io.go:86:11:86:19 | selection of Stdout |
| io.go:89:26:89:38 | "some string" | io.go:89:8:89:39 | call to NewReader |
Expand Down
18 changes: 12 additions & 6 deletions go/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,15 @@ edges
| passwords.go:34:28:34:35 | password | passwords.go:34:14:34:35 | ...+... | provenance | Config |
| passwords.go:36:2:36:5 | definition of obj1 | passwords.go:39:14:39:17 | obj1 | provenance | |
| passwords.go:36:2:36:5 | definition of obj1 | passwords.go:39:14:39:17 | obj1 | provenance | |
| passwords.go:36:10:38:2 | struct literal | passwords.go:36:2:36:5 | definition of obj1 | provenance | |
| passwords.go:36:10:38:2 | struct literal | passwords.go:39:14:39:17 | obj1 | provenance | |
| passwords.go:36:10:38:2 | struct literal | passwords.go:39:14:39:17 | obj1 | provenance | |
| passwords.go:37:13:37:13 | x | passwords.go:36:10:38:2 | struct literal | provenance | Config |
| passwords.go:39:2:39:18 | []type{args} [array] | passwords.go:36:2:36:5 | definition of obj1 | provenance | |
| passwords.go:39:14:39:17 | obj1 | passwords.go:39:2:39:18 | []type{args} [array] | provenance | |
| passwords.go:41:2:41:5 | definition of obj2 | passwords.go:44:14:44:17 | obj2 | provenance | |
| passwords.go:41:2:41:5 | definition of obj2 | passwords.go:44:14:44:17 | obj2 | provenance | |
| passwords.go:41:10:43:2 | struct literal | passwords.go:41:2:41:5 | definition of obj2 | provenance | |
| passwords.go:41:10:43:2 | struct literal | passwords.go:44:14:44:17 | obj2 | provenance | |
| passwords.go:41:10:43:2 | struct literal | passwords.go:44:14:44:17 | obj2 | provenance | |
| passwords.go:42:6:42:13 | password | passwords.go:41:10:43:2 | struct literal | provenance | Config |
| passwords.go:44:2:44:18 | []type{args} [array] | passwords.go:41:2:41:5 | definition of obj2 | provenance | |
| passwords.go:44:14:44:17 | obj2 | passwords.go:44:2:44:18 | []type{args} [array] | provenance | |
Expand All @@ -83,7 +85,8 @@ edges
| passwords.go:48:11:48:18 | password | passwords.go:46:6:46:9 | definition of obj3 | provenance | Config |
| passwords.go:85:2:85:14 | definition of utilityObject | passwords.go:88:14:88:26 | utilityObject | provenance | |
| passwords.go:85:2:85:14 | definition of utilityObject | passwords.go:88:14:88:26 | utilityObject | provenance | |
| passwords.go:85:19:87:2 | struct literal | passwords.go:85:2:85:14 | definition of utilityObject | provenance | |
| passwords.go:85:19:87:2 | struct literal | passwords.go:88:14:88:26 | utilityObject | provenance | |
| passwords.go:85:19:87:2 | struct literal | passwords.go:88:14:88:26 | utilityObject | provenance | |
| passwords.go:86:16:86:36 | call to make | passwords.go:85:19:87:2 | struct literal | provenance | Config |
| passwords.go:88:2:88:27 | []type{args} [array] | passwords.go:85:2:85:14 | definition of utilityObject | provenance | |
| passwords.go:88:14:88:26 | utilityObject | passwords.go:88:2:88:27 | []type{args} [array] | provenance | |
Expand All @@ -99,9 +102,12 @@ edges
| passwords.go:118:2:118:7 | definition of config [x] | passwords.go:126:14:126:19 | config [x] | provenance | |
| passwords.go:118:2:118:7 | definition of config [y] | passwords.go:125:14:125:19 | config [y] | provenance | |
| passwords.go:118:2:118:7 | definition of config [y] | passwords.go:127:14:127:19 | config [y] | provenance | |
| passwords.go:118:12:123:2 | struct literal | passwords.go:118:2:118:7 | definition of config | provenance | |
| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:118:2:118:7 | definition of config [x] | provenance | |
| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:118:2:118:7 | definition of config [y] | provenance | |
| passwords.go:118:12:123:2 | struct literal | passwords.go:125:14:125:19 | config | provenance | |
| passwords.go:118:12:123:2 | struct literal | passwords.go:125:14:125:19 | config | provenance | |
| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:125:14:125:19 | config [x] | provenance | |
| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:126:14:126:19 | config [x] | provenance | |
| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:125:14:125:19 | config [y] | provenance | |
| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:127:14:127:19 | config [y] | provenance | |
| passwords.go:119:13:119:13 | x | passwords.go:118:12:123:2 | struct literal | provenance | Config |
| passwords.go:121:13:121:20 | password | passwords.go:118:12:123:2 | struct literal | provenance | Config |
| passwords.go:121:13:121:20 | password | passwords.go:118:12:123:2 | struct literal [x] | provenance | |
Expand Down
Loading