Skip to content

Commit c9b49d3

Browse files
KwstubbsKwstubbs
committed
resolve feedback
1 parent 530c76c commit c9b49d3

File tree

2 files changed

+23
-22
lines changed

2 files changed

+23
-22
lines changed

‎go/ql/lib/semmle/go/security/TaintedPathCustomizations.qll

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import go
77
import semmle.go.dataflow.barrierguardutil.RegexpCheck
8-
8+
import DataFlow
99
/**
1010
* Provides extension points for customizing the taint tracking configuration for reasoning about
1111
* path-traversal vulnerabilities.
@@ -100,17 +100,12 @@ module TaintedPath {
100100
}
101101
}
102102

103-
/** An call to ParseMultipartForm creates multipart.Form and cleans mutlpart.Form.FileHeader.Filename using path.Base() */
103+
/**An call to ParseMultipartForm creates multipart.Form and cleans mutlpart.Form.FileHeader.Filename using path.Base() */
104104
class MultipartClean extends Sanitizer {
105105
MultipartClean() {
106-
exists(DataFlow::FieldReadNode frn, ControlFlow::Node node, DataFlow::CallNode cleanCall, Method get |
107-
get.hasQualifiedName("net/http","Request", "ParseMultipartForm") and
108-
cleanCall = get.getACall() and
109-
cleanCall.asInstruction() = node and
106+
exists(DataFlow::FieldReadNode frn |
110107
frn.getField().hasQualifiedName("mime/multipart", "FileHeader", "Filename") and
111-
node.getASuccessor*() = frn.asInstruction()
112-
|
113-
this = frn.getBase()
108+
this = frn
114109
)
115110
}
116111
}
@@ -133,7 +128,7 @@ module TaintedPath {
133128
}
134129
}
135130
/**
136-
* A replacement of the form `!strings.ReplaceAll(nd, "..")` or `!strings.ReplaceAll(nd, ".")`, considered as a sanitizer guard for
131+
* A replacement of the form `!strings.ReplaceAll(nd, "..")` or `!strings.ReplaceAll(nd, ".")`, considered as a sanitizer for
137132
* path traversal.
138133
*/
139134
class DotDotReplace extends Sanitizer {
@@ -142,7 +137,7 @@ module TaintedPath {
142137
cleanCall =
143138
any(Function f | f.hasQualifiedName("strings", "ReplaceAll")).getACall() and
144139
valueNode = cleanCall.getArgument(1) and
145-
(valueNode.asExpr().(StringLit).getValue() = ".." or valueNode.asExpr().(StringLit).getValue() = ".") and
140+
valueNode.asExpr().(StringLit).getValue() = ["..", "."] and
146141
this = cleanCall.getResult()
147142
)
148143
}

‎go/ql/test/query-tests/Security/CWE-022/TaintedPath.expected

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
edges
22
<<<<<<< HEAD
3+
<<<<<<< HEAD
34
| TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:13:18:13:30 | call to Query | provenance | |
45
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:16:29:16:40 | tainted_path | provenance | |
56
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:20:57:20:68 | tainted_path | provenance | |
@@ -12,24 +13,29 @@ edges
1213
| TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:77:28:77:56 | call to Base |
1314
| tst.go:14:2:14:39 | ... := ...[1] : pointer type | tst.go:17:41:17:56 | selection of Filename |
1415
>>>>>>> a45343fb6c (Add New Sanitizers and Modify Old Ones)
16+
=======
17+
| TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:13:18:13:30 | call to Query |
18+
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:16:29:16:40 | tainted_path |
19+
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:20:57:20:68 | tainted_path |
20+
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:67:39:67:56 | ...+... |
21+
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:77:38:77:55 | ...+... |
22+
| TaintedPath.go:20:57:20:68 | tainted_path | TaintedPath.go:20:28:20:69 | call to Join |
23+
| TaintedPath.go:67:39:67:56 | ...+... | TaintedPath.go:67:28:67:57 | call to Clean |
24+
| TaintedPath.go:77:38:77:55 | ...+... | TaintedPath.go:77:28:77:56 | call to Base |
25+
>>>>>>> db14838a4f (resolve feedback)
1526
nodes
1627
| TaintedPath.go:13:18:13:22 | selection of URL | semmle.label | selection of URL |
1728
| TaintedPath.go:13:18:13:30 | call to Query | semmle.label | call to Query |
1829
| TaintedPath.go:16:29:16:40 | tainted_path | semmle.label | tainted_path |
1930
| TaintedPath.go:20:28:20:69 | call to Join | semmle.label | call to Join |
20-
<<<<<<< HEAD
2131
| TaintedPath.go:20:57:20:68 | tainted_path | semmle.label | tainted_path |
22-
| tst.go:14:2:14:39 | ... := ...[1] | semmle.label | ... := ...[1] |
23-
=======
2432
| TaintedPath.go:67:28:67:57 | call to Clean | semmle.label | call to Clean |
33+
| TaintedPath.go:67:39:67:56 | ...+... | semmle.label | ...+... |
2534
| TaintedPath.go:77:28:77:56 | call to Base | semmle.label | call to Base |
26-
| tst.go:14:2:14:39 | ... := ...[1] : pointer type | semmle.label | ... := ...[1] : pointer type |
27-
>>>>>>> a45343fb6c (Add New Sanitizers and Modify Old Ones)
28-
| tst.go:17:41:17:56 | selection of Filename | semmle.label | selection of Filename |
35+
| TaintedPath.go:77:38:77:55 | ...+... | semmle.label | ...+... |
2936
subpaths
3037
#select
31-
| TaintedPath.go:16:29:16:40 | tainted_path | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:16:29:16:40 | tainted_path | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
32-
| TaintedPath.go:20:28:20:69 | call to Join | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:20:28:20:69 | call to Join | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
33-
| TaintedPath.go:67:28:67:57 | call to Clean | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:67:28:67:57 | call to Clean | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
34-
| TaintedPath.go:77:28:77:56 | call to Base | TaintedPath.go:13:18:13:22 | selection of URL : pointer type | TaintedPath.go:77:28:77:56 | call to Base | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
35-
| tst.go:17:41:17:56 | selection of Filename | tst.go:14:2:14:39 | ... := ...[1] : pointer type | tst.go:17:41:17:56 | selection of Filename | This path depends on a $@. | tst.go:14:2:14:39 | ... := ...[1] | user-provided value |
38+
| TaintedPath.go:16:29:16:40 | tainted_path | TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:16:29:16:40 | tainted_path | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
39+
| TaintedPath.go:20:28:20:69 | call to Join | TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:20:28:20:69 | call to Join | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
40+
| TaintedPath.go:67:28:67:57 | call to Clean | TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:67:28:67:57 | call to Clean | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |
41+
| TaintedPath.go:77:28:77:56 | call to Base | TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:77:28:77:56 | call to Base | This path depends on a $@. | TaintedPath.go:13:18:13:22 | selection of URL | user-provided value |

0 commit comments

Comments
 (0)