mirror of
https://github.com/github/codeql.git
synced 2026-01-29 14:23:03 +01:00
More cleanup in help and tests.
In particular, I have copied over the examples referenced in the qhelp into the test folder and made sure they compile.
This commit is contained in:
@@ -2,26 +2,25 @@
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>
|
||||
Testing untrusted user input against untrusted user input results in
|
||||
a bypass of the conditional check as the attacker may modify parameters to match.
|
||||
</p>
|
||||
Conditional checks that compare two values that are both controlled by an untrusted user against
|
||||
each other are easy to bypass and should not be used in security-critical contexts.
|
||||
</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>
|
||||
To guard against this, it is advisable to avoid framing a comparison
|
||||
where both sides are untrusted user inputs.
|
||||
Instead, use a configuration to store and access the values required.
|
||||
</p>
|
||||
To guard against bypass, it is advisable to avoid framing a comparison where both sides are
|
||||
untrusted user inputs. Instead, use a configuration to store and access the values required.
|
||||
</p>
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>
|
||||
The following example shows a comparison where both the sides
|
||||
are from attacker-controlled request headers. This should be avoided.:
|
||||
</p>
|
||||
The following example shows a comparison where both the sides are from attacker-controlled request
|
||||
headers. This should be avoided:
|
||||
</p>
|
||||
<sample src="ConditionalBypassBad.go" />
|
||||
<p>
|
||||
One way to remedy the problem is to test against a value stored in a configuration:
|
||||
</p>
|
||||
</p>
|
||||
<sample src="ConditionalBypassGood.go" />
|
||||
</example>
|
||||
</qhelp>
|
||||
@@ -4,8 +4,8 @@ import (
|
||||
"net/http"
|
||||
)
|
||||
|
||||
func ex1(w http.ResponseWriter, r *http.Request) {
|
||||
// bad the origin and the host headers are user controlled
|
||||
func exampleHandlerBad(w http.ResponseWriter, r *http.Request) {
|
||||
// BAD: the Origin and Host headers are user controlled
|
||||
if r.Header.Get("Origin") != "http://"+r.Host {
|
||||
//do something
|
||||
}
|
||||
|
||||
@@ -4,7 +4,8 @@ import (
|
||||
"net/http"
|
||||
)
|
||||
|
||||
func ex1(w http.ResponseWriter, r *http.Request) {
|
||||
func exampleHandlerGood(w http.ResponseWriter, r *http.Request) {
|
||||
// GOOD: the configuration is not user controlled
|
||||
if r.Header.Get("Origin") != config.get("Host") {
|
||||
//do something
|
||||
}
|
||||
|
||||
@@ -1,2 +1,3 @@
|
||||
| ConditionalBypassBad.go:9:5:9:46 | ...!=... | This comparison compares user-controlled values from $@ and $@, and hence can be bypassed. | ConditionalBypassBad.go:9:5:9:12 | selection of Header : Header | here | ConditionalBypassBad.go:9:41:9:46 | selection of Host : string | here |
|
||||
| condition.go:9:5:9:46 | ...!=... | This comparison compares user-controlled values from $@ and $@, and hence can be bypassed. | condition.go:9:5:9:12 | selection of Header : Header | here | condition.go:9:41:9:46 | selection of Host : string | here |
|
||||
| condition.go:16:5:16:62 | ...!=... | This comparison compares user-controlled values from $@ and $@, and hence can be bypassed. | condition.go:16:5:16:12 | selection of Header : Header | here | condition.go:16:41:16:48 | selection of Header : Header | here |
|
||||
|
||||
12
ql/test/experimental/CWE-840/ConditionalBypassBad.go
Normal file
12
ql/test/experimental/CWE-840/ConditionalBypassBad.go
Normal file
@@ -0,0 +1,12 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
)
|
||||
|
||||
func exampleHandlerBad(w http.ResponseWriter, r *http.Request) {
|
||||
// BAD: the Origin and Host headers are user controlled
|
||||
if r.Header.Get("Origin") != "http://"+r.Host {
|
||||
//do something
|
||||
}
|
||||
}
|
||||
12
ql/test/experimental/CWE-840/ConditionalBypassGood.go
Normal file
12
ql/test/experimental/CWE-840/ConditionalBypassGood.go
Normal file
@@ -0,0 +1,12 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
)
|
||||
|
||||
func exampleHandlerGood(w http.ResponseWriter, r *http.Request) {
|
||||
// GOOD: the configuration is not user controlled
|
||||
if r.Header.Get("Origin") != config.get("Host") {
|
||||
//do something
|
||||
}
|
||||
}
|
||||
@@ -4,21 +4,21 @@ import (
|
||||
"net/http"
|
||||
)
|
||||
|
||||
// bad : taken from https://www.gorillatoolkit.org/pkg/websocket
|
||||
// BAD: taken from https://www.gorillatoolkit.org/pkg/websocket
|
||||
func ex1(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Header.Get("Origin") != "http://"+r.Host {
|
||||
//do something
|
||||
}
|
||||
}
|
||||
|
||||
// bad both are from remote sources
|
||||
// BAD: both operands are from remote sources
|
||||
func ex2(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Header.Get("Origin") != "http://"+r.Header.Get("Header") {
|
||||
//do something
|
||||
}
|
||||
}
|
||||
|
||||
// good
|
||||
// GOOD
|
||||
func ex3(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Header.Get("Origin") != "http://"+"test" {
|
||||
//do something
|
||||
|
||||
9
ql/test/experimental/CWE-840/util.go
Normal file
9
ql/test/experimental/CWE-840/util.go
Normal file
@@ -0,0 +1,9 @@
|
||||
package main
|
||||
|
||||
type Config struct{}
|
||||
|
||||
func (_ Config) get(s string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
var config = Config{}
|
||||
Reference in New Issue
Block a user