Go: Make models-as-data source models for variadic parameters work #18275
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Source models defined using models-as-data now work for variadic parameters. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,24 +5,36 @@ | |
| | 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 | "&" | 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] | | ||
|
|
@@ -39,17 +51,25 @@ | |
| | 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 | | ||
|
|
@@ -58,27 +78,35 @@ | |
| | 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 | | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,7 +44,13 @@ 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" | ||
|
|
||
| test.VariadicSink(source()) // $ hasTaintFlow="[]type{args}" | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,9 +10,13 @@ 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 | | ||
|
|
@@ -27,8 +31,10 @@ 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 | | ||
|
|
@@ -46,8 +52,14 @@ 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 | | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,15 +66,13 @@ 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: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: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 | | | ||
|
|
@@ -85,8 +83,7 @@ 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: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 | | | ||
|
|
@@ -102,12 +99,9 @@ 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: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 | | | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this defined here and not in localAdditionalTaintStep?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In increasing order of how good an answer it is:
(a) It's what they do in java.
(b) I copied it before I realised that it wouldn't affect global taint flow at all.
(c) I think it is helpful. Six months ago I tried to convert old-style models for variadic functions to MaD. One of the sticking points was "Some barrier guard definitions use localTaintStep to detect taint flow, but it doesn't work flow through an implicit varargs slice." With this added (for container reads and container writes) we get that
localTaintincludes flow into and out of arrays, including through implicit varargs slices.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think I see: you want both reads and stores to appear according to the handy utility
localTaintStep, but only reads to be contributed to the additional-taint-step for global dataflow's purposes?If so, suggest three things:
localAdditionalTaintStepis already included -- instead just comment that the read steps are already taken care ofUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
summaryGetterStepsaysNote also that something related is included in
readStep(andstoreStep).2. Done.
3. I'm not exactly sure what to say. I've put "Treat container read steps as taint for global taint flow." @aschackmull can you suggest a more detailed explanation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the qldoc that Owen referenced says, the summary-getter step should not be passed to global data flow in any form (neither as a read step nor as a taint step). To elaborate: Container read steps should be passed to data flow as taint steps. But read steps and summary-getter steps are separate things: A read step is atomic, whereas a summary-getter step represents a flow path through a callable with a nested read step, such that the entire thing can be summarized and treated as a read step in those contexts that aren't capable of doing such summarization. And data flow notably is perfectly capable of such summarization, so it only needs to know about the atomic read steps.