hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-01-29 14:23:03 +01:00
Code Issues Packages Projects Releases Wiki Activity

ReflectedXss: Remove FPs from constant prefix Fprintfs

Browse Source
This commit is contained in:
Sauyon Lee
2020-02-03 13:59:36 -08:00
parent 3c88eab84c
commit 87865afa42
7 changed files with 51 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
ql/src/Security/CWE-079/ReflectedXss.go
View File

@@ -11,7 +11,7 @@ func serve() {
username := r.Form.Get("username")
if !isValidUsername(username) {
// BAD: a request parameter is incorporated without validation into the response
fmt.Fprintf(w, "Unknown user: %q", username)
fmt.Fprintf(w, "%q is an unknown user", username)
} else {
// TODO: do something exciting
}

2
ql/src/Security/CWE-079/ReflectedXssGood.go
View File

@@ -12,7 +12,7 @@ func serve1() {
username := r.Form.Get("username")
if !isValidUsername(username) {
// BAD: a request parameter is incorporated without validation into the response
fmt.Fprintf(w, "Unknown user: %q", html.EscapeString(username))
fmt.Fprintf(w, "%q is an unknown user", html.EscapeString(username))
} else {
// TODO: do something exciting
}

16
ql/src/semmle/go/security/ReflectedXssCustomizations.qll
View File

@@ -37,18 +37,28 @@ module ReflectedXss {
* is to prevent us from flagging plain-text or JSON responses as vulnerable.
*/
class HttpResponseBodySink extends Sink, HTTP::ResponseBody {
HttpResponseBodySink() { not nonHtmlContentType(this.getResponseWriter()) }
HttpResponseBodySink() { not nonHtmlContentType(this) }
}
/**
* Holds if `h` may send a response with a content type other than HTML.
*/
private predicate nonHtmlContentType(HTTP::ResponseWriter rw) {
private predicate nonHtmlContentType(HTTP::ResponseBody body) {
exists(HTTP::HeaderWrite hw |
hw = rw.getAHeaderWrite() and hw.definesHeader("content-type", _)
hw = body.getResponseWriter().getAHeaderWrite() and hw.definesHeader("content-type", _)
|
not exists(string tp | hw.definesHeader("content-type", tp) | tp.regexpMatch("(?i).*html.*"))
)
or
not exists(HTTP::HeaderWrite hw, string tp |
hw = body.getResponseWriter().getAHeaderWrite() and hw.definesHeader("content-type", tp)
|
tp.regexpMatch("(?i).*html.*")
) and
exists(DataFlow::CallNode call | call.getTarget().hasQualifiedName("fmt", "Fprintf") |
body = call.getAnArgument() and
call.getArgument(1).getStringValue().regexpMatch("^[^<%].*")
)
}
/**

18
ql/test/query-tests/Security/CWE-079/ReflectedXss.expected
View File

@@ -1,11 +1,15 @@
edges
| ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:39:14:46 | username |
| contenttype.go:10:11:10:16 | selection of Form : Values | contenttype.go:16:11:16:22 | type conversion |
| ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:44:14:51 | username |
| 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 |
nodes
| ReflectedXss.go:11:15:11:20 | selection of Form : Values | semmle.label | selection of Form : Values |
| ReflectedXss.go:14:39:14:46 | username | semmle.label | username |
| contenttype.go:10:11:10:16 | selection of Form : Values | semmle.label | selection of Form : Values |
| contenttype.go:16:11:16:22 | type conversion | semmle.label | type conversion |
| ReflectedXss.go:14:44:14:51 | username | semmle.label | username |
| 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 |
| contenttype.go:53:34:53:37 | data | semmle.label | data |
#select
| ReflectedXss.go:14:39:14:46 | username | ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:39:14:46 | username | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:11:15:11:20 | selection of Form | user-provided value |
| contenttype.go:16:11:16:22 | type conversion | contenttype.go:10:11:10:16 | selection of Form : Values | contenttype.go:16:11:16:22 | type conversion | Cross-site scripting vulnerability due to $@. | contenttype.go:10:11:10:16 | selection of Form | user-provided value |
| ReflectedXss.go:14:44:14:51 | username | ReflectedXss.go:11:15:11:20 | selection of Form : Values | ReflectedXss.go:14:44:14:51 | username | Cross-site scripting vulnerability due to $@. | ReflectedXss.go:11:15:11:20 | selection of Form | user-provided value |
| 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: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 |

2
ql/test/query-tests/Security/CWE-079/ReflectedXss.go
View File

@@ -11,7 +11,7 @@ func serve() {
username := r.Form.Get("username")
if !isValidUsername(username) {
// BAD: a request parameter is incorporated without validation into the response
fmt.Fprintf(w, "Unknown user: %q", username)
fmt.Fprintf(w, "%q is an unknown user", username)
} else {
// TODO: do something exciting
}

2
ql/test/query-tests/Security/CWE-079/ReflectedXssGood.go
View File

@@ -12,7 +12,7 @@ func serve1() {
username := r.Form.Get("username")
if !isValidUsername(username) {
// BAD: a request parameter is incorporated without validation into the response
fmt.Fprintf(w, "Unknown user: %q", html.EscapeString(username))
fmt.Fprintf(w, "%q is an unknown user", html.EscapeString(username))
} else {
// TODO: do something exciting
}

23
ql/test/query-tests/Security/CWE-079/contenttype.go
View File

@@ -1,6 +1,7 @@
package main
import (
"fmt"
"net/http"
)
@@ -31,3 +32,25 @@ func serve3() {
})
http.ListenAndServe(":80", nil)
}
func serve4() {
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
r.ParseForm()
data := r.Form.Get("data")
fmt.Fprintf(w, "Constant: %s", data) // OK; the prefix causes the content type header to be text/plain
})
http.ListenAndServe(":80", nil)
}
func serve5() {
http.HandleFunc("/echo", func(w http.ResponseWriter, r *http.Request) {
r.ParseForm()
data := r.Form.Get("data")
w.Header().Set("Content-Type", "text/html")
fmt.Fprintf(w, "Constant: %s", data) // Not OK; the content-type header is explicitly set to html
})
http.ListenAndServe(":80", nil)
}