mirror of
https://github.com/github/codeql.git
synced 2026-01-29 22:32:58 +01:00
Improve net/http taint-tracking fidelity
* Don't taint error returns from http.Request methods * Track taint across mime/multipart.Part methods
This commit is contained in:
Chris Smowton
@@ -34,6 +34,18 @@ module MimeMultipart {
|
||||
hasQualifiedName("mime/multipart", "FileHeader", "Open") and
|
||||
(inp.isReceiver() and outp.isResult(0))
|
||||
or
|
||||
// signature: func (*Part).FileName() string
|
||||
hasQualifiedName("mime/multipart", "Part", "FileName") and
|
||||
(inp.isReceiver() and outp.isResult(0))
|
||||
or
|
||||
// signature: func (*Part).FormName() string
|
||||
hasQualifiedName("mime/multipart", "Part", "FormName") and
|
||||
(inp.isReceiver() and outp.isResult(0))
|
||||
or
|
||||
// signature: func (*Part).Read([]byte) (int, error)
|
||||
hasQualifiedName("mime/multipart", "Part", "FormName") and
|
||||
(inp.isReceiver() and outp.isParameter(0))
|
||||
or
|
||||
// signature: func (*Reader).NextPart() (*Part, error)
|
||||
hasQualifiedName("mime/multipart", "Reader", "NextPart") and
|
||||
(inp.isReceiver() and outp.isResult(0))
|
||||
|
||||
@@ -23,18 +23,20 @@ module NetHttp {
|
||||
}
|
||||
}
|
||||
|
||||
private class UserControlledRequestMethod extends UntrustedFlowSource::Range,
|
||||
DataFlow::MethodCallNode {
|
||||
private class UserControlledRequestMethod extends UntrustedFlowSource::Range {
|
||||
UserControlledRequestMethod() {
|
||||
exists(string methName | this.getTarget().hasQualifiedName("net/http", "Request", methName) |
|
||||
methName = "Cookie" or
|
||||
methName = "Cookies" or
|
||||
methName = "FormFile" or
|
||||
methName = "FormValue" or
|
||||
methName = "MultipartReader" or
|
||||
methName = "PostFormValue" or
|
||||
methName = "Referer" or
|
||||
methName = "UserAgent"
|
||||
exists(DataFlow::MethodCallNode callNode, string methName, int resultIdx |
|
||||
callNode.getTarget().hasQualifiedName("net/http", "Request", methName) and
|
||||
this = callNode.getResult(resultIdx)
|
||||
|
|
||||
methName =
|
||||
[
|
||||
"Cookie", "Cookies", "FormValue", "MultipartReader", "PostFormValue", "Referer",
|
||||
"UserAgent"
|
||||
] and
|
||||
resultIdx = 0
|
||||
or
|
||||
methName = "FormFile" and resultIdx = [0, 1]
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,13 @@
|
||||
edges
|
||||
| ReflectedXss.go:12:15:12:20 | selection of Form : Values | ReflectedXss.go:15:44:15:51 | username |
|
||||
| ReflectedXss.go:13:15:13:20 | selection of Form : Values | ReflectedXss.go:16:44:16:51 | username |
|
||||
| ReflectedXss.go:48:2:48:38 | ... := ...[0] : pointer type | ReflectedXss.go:49:10:49:57 | type conversion |
|
||||
| ReflectedXss.go:52:2:52:44 | ... := ...[0] : File | ReflectedXss.go:54:10:54:57 | type conversion |
|
||||
| ReflectedXss.go:52:2:52:44 | ... := ...[1] : pointer type | ReflectedXss.go:55:10:55:62 | type conversion |
|
||||
| ReflectedXss.go:52:2:52:44 | ... := ...[1] : pointer type | ReflectedXss.go:55:46:55:51 | implicit dereference : FileHeader |
|
||||
| ReflectedXss.go:55:46:55:51 | implicit dereference : FileHeader | ReflectedXss.go:55:10:55:62 | type conversion |
|
||||
| ReflectedXss.go:55:46:55:51 | implicit dereference : FileHeader | ReflectedXss.go:55:46:55:51 | implicit dereference : FileHeader |
|
||||
| ReflectedXss.go:59:2:59:35 | ... := ...[0] : pointer type | ReflectedXss.go:65:10:65:55 | type conversion |
|
||||
| ReflectedXss.go:59:2:59:35 | ... := ...[0] : pointer type | ReflectedXss.go:66:10:66:18 | byteSlice |
|
||||
| contenttype.go:11:11:11:16 | selection of Form : Values | contenttype.go:17:11:17:22 | type conversion |
|
||||
| contenttype.go:49:11:49:16 | selection of Form : Values | contenttype.go:53:34:53:37 | data |
|
||||
| contenttype.go:63:10:63:28 | call to FormValue : string | contenttype.go:64:52:64:55 | data |
|
||||
@@ -15,8 +23,18 @@ edges
|
||||
| websocketXss.go:50:3:50:10 | definition of gorilla2 : slice type | websocketXss.go:52:24:52:31 | gorilla2 |
|
||||
| websocketXss.go:54:3:54:38 | ... := ...[1] : slice type | websocketXss.go:55:24:55:31 | gorilla3 |
|
||||
nodes
|
||||
| ReflectedXss.go:12:15:12:20 | selection of Form : Values | semmle.label | selection of Form : Values |
|
||||
| ReflectedXss.go:15:44:15:51 | username | semmle.label | username |
|
||||
| ReflectedXss.go:13:15:13:20 | selection of Form : Values | semmle.label | selection of Form : Values |
|
||||
| ReflectedXss.go:16:44:16:51 | username | semmle.label | username |
|
||||
| ReflectedXss.go:48:2:48:38 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
|
||||
| ReflectedXss.go:49:10:49:57 | type conversion | semmle.label | type conversion |
|
||||
| ReflectedXss.go:52:2:52:44 | ... := ...[0] : File | semmle.label | ... := ...[0] : File |
|
||||
| ReflectedXss.go:52:2:52:44 | ... := ...[1] : pointer type | semmle.label | ... := ...[1] : pointer type |
|
||||
| ReflectedXss.go:54:10:54:57 | type conversion | semmle.label | type conversion |
|
||||
| ReflectedXss.go:55:10:55:62 | type conversion | semmle.label | type conversion |
|
||||
| ReflectedXss.go:55:46:55:51 | implicit dereference : FileHeader | semmle.label | implicit dereference : FileHeader |
|
||||
| ReflectedXss.go:59:2:59:35 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type |
|
||||
| ReflectedXss.go:65:10:65:55 | type conversion | semmle.label | type conversion |
|
||||
| ReflectedXss.go:66:10:66:18 | byteSlice | semmle.label | byteSlice |
|
||||
| contenttype.go:11:11:11:16 | selection of Form : Values | semmle.label | selection of Form : Values |
|
||||
| contenttype.go:17:11:17:22 | type conversion | semmle.label | type conversion |
|
||||
| contenttype.go:49:11:49:16 | selection of Form : Values | semmle.label | selection of Form : Values |
|
||||
@@ -46,7 +64,12 @@ nodes
|
||||
| websocketXss.go:54:3:54:38 | ... := ...[1] : slice type | semmle.label | ... := ...[1] : slice type |
|
||||
| websocketXss.go:55:24:55:31 | gorilla3 | semmle.label | gorilla3 |
|
||||
#select
|
||||
| ReflectedXss.go:15:44:15:51 | username | ReflectedXss.go:12:15:12:20 | selection of Form : Values | ReflectedXss.go:15:44:15:51 | username | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:12:15:12:20 | selection of Form | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| ReflectedXss.go:16:44:16:51 | username | ReflectedXss.go:13:15:13:20 | selection of Form : Values | ReflectedXss.go:16:44:16:51 | username | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:13:15:13:20 | selection of Form | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| ReflectedXss.go:49:10:49:57 | type conversion | ReflectedXss.go:48:2:48:38 | ... := ...[0] : pointer type | ReflectedXss.go:49:10:49:57 | type conversion | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:48:2:48:38 | ... := ...[0] | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| ReflectedXss.go:54:10:54:57 | type conversion | ReflectedXss.go:52:2:52:44 | ... := ...[0] : File | ReflectedXss.go:54:10:54:57 | type conversion | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:52:2:52:44 | ... := ...[0] | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| ReflectedXss.go:55:10:55:62 | type conversion | ReflectedXss.go:52:2:52:44 | ... := ...[1] : pointer type | ReflectedXss.go:55:10:55:62 | type conversion | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:52:2:52:44 | ... := ...[1] | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| ReflectedXss.go:65:10:65:55 | type conversion | ReflectedXss.go:59:2:59:35 | ... := ...[0] : pointer type | ReflectedXss.go:65:10:65:55 | type conversion | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:59:2:59:35 | ... := ...[0] | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| ReflectedXss.go:66:10:66:18 | byteSlice | ReflectedXss.go:59:2:59:35 | ... := ...[0] : pointer type | ReflectedXss.go:66:10:66:18 | byteSlice | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:59:2:59:35 | ... := ...[0] | user-provided value | ReflectedXss.go:0:0:0:0 | ReflectedXss.go | |
|
||||
| contenttype.go:17:11:17:22 | type conversion | contenttype.go:11:11:11:16 | selection of Form : Values | contenttype.go:17:11:17:22 | type conversion | Cross-site scripting vulnerability due to $@. | contenttype.go:11:11:11:16 | selection of Form | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | |
|
||||
| contenttype.go:53:34:53:37 | data | contenttype.go:49:11:49:16 | selection of Form : Values | contenttype.go:53:34:53:37 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:49:11:49:16 | selection of Form | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | |
|
||||
| contenttype.go:64:52:64:55 | data | contenttype.go:63:10:63:28 | call to FormValue : string | contenttype.go:64:52:64:55 | data | Cross-site scripting vulnerability due to $@. | contenttype.go:63:10:63:28 | call to FormValue | user-provided value | contenttype.go:0:0:0:0 | contenttype.go | |
|
||||
|
||||
@@ -3,6 +3,7 @@ package main
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
)
|
||||
|
||||
@@ -41,3 +42,29 @@ func ServeJsonDirect(w http.ResponseWriter, r http.Request) {
|
||||
w.Write(noLongerTainted)
|
||||
|
||||
}
|
||||
|
||||
func ErrTest(w http.ResponseWriter, r http.Request) {
|
||||
|
||||
cookie, err := r.Cookie("somecookie")
|
||||
w.Write([]byte(fmt.Sprintf("Cookie result: %v", cookie))) // BAD: Cookie's value is user-controlled
|
||||
w.Write([]byte(fmt.Sprintf("Cookie check error: %v", err))) // GOOD: Cookie's err return is harmless
|
||||
|
||||
file, header, err := r.FormFile("someFile")
|
||||
content, err2 := ioutil.ReadAll(file)
|
||||
w.Write([]byte(fmt.Sprintf("File content: %v", content))) // BAD: file content is user-controlled
|
||||
w.Write([]byte(fmt.Sprintf("File name: %v", header.Filename))) // BAD: file header is user-controlled
|
||||
w.Write([]byte(fmt.Sprintf("FormFile error: %v", err))) // GOOD: FormFile's err return is harmless
|
||||
w.Write([]byte(fmt.Sprintf("FormFile error: %v", err2))) // GOOD: ReadAll's err return is harmless
|
||||
|
||||
reader, err := r.MultipartReader()
|
||||
part, err2 := reader.NextPart()
|
||||
partName := part.FileName()
|
||||
byteSlice := make([]byte, 100)
|
||||
part.Read(byteSlice)
|
||||
|
||||
w.Write([]byte(fmt.Sprintf("Part name: %v", partName))) // BAD: part name is user-controlled
|
||||
w.Write(byteSlice) // BAD: part contents are user-controlled
|
||||
w.Write([]byte(fmt.Sprintf("MultipartReader error: %v", err))) // GOOD: MultipartReader's err return is harmless
|
||||
w.Write([]byte(fmt.Sprintf("MultipartReader error: %v", err2))) // GOOD: NextPart's err return is harmless
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user