Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit ef7198c

Browse files
gagliardettogagliardetto
committed
Improve query scenarios
1 parent 282f7af commit ef7198c

3 files changed

Lines changed: 165 additions & 9 deletions

File tree

‎ql/src/experimental/CWE-352/ConstantOauth2State.ql‎

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,19 @@
1212
import go
1313
import DataFlow::PathGraph
1414

15-
/*
15+
/**
1616
* A method that creates a new URL that will send the user
1717
* to the oauth2 authorization dialog of the provider.
1818
*/
19-
2019
class AuthCodeURL extends Method {
2120
AuthCodeURL() { this.hasQualifiedName("golang.org/x/oauth2", "Config", "AuthCodeURL") }
2221
}
2322

24-
/*
23+
/**
2524
* A flow of a constant string value to a call to AuthCodeURL as the
2625
* `state` parameter.
2726
*/
28-
29-
class ConstantStateFlowConf extends TaintTracking::Configuration {
27+
class ConstantStateFlowConf extends DataFlow::Configuration {
3028
ConstantStateFlowConf() { this = "ConstantStateFlowConf" }
3129

3230
predicate isSource(DataFlow::Node source, Literal state) {
@@ -42,7 +40,54 @@ class ConstantStateFlowConf extends TaintTracking::Configuration {
4240
override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
4341
}
4442

45-
from ConstantStateFlowConf cfg, DataFlow::PathNode source, DataFlow::PathNode sink
46-
where cfg.hasFlowPath(source, sink)
43+
/** A flow to a printer function of the fmt package. */
44+
class FlowToPrint extends DataFlow::Configuration {
45+
FlowToPrint() { this = "FlowToPrint" }
46+
47+
predicate isSource(DataFlow::Node source, DataFlow::CallNode call) {
48+
exists(AuthCodeURL m | call = m.getACall() | source = call.getResult())
49+
}
50+
51+
predicate isSink(DataFlow::Node sink, DataFlow::CallNode call) {
52+
exists(Fmt::Printer printer | call = printer.getACall() | sink = call.getArgument(_))
53+
}
54+
55+
override predicate isSource(DataFlow::Node source) { isSource(source, _) }
56+
57+
override predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
58+
}
59+
60+
/** Holds if the provided CallNode's result flows to a Printer call as argument. */
61+
predicate flowsToPrinter(DataFlow::CallNode authCodeURLCall) {
62+
exists(FlowToPrint cfg, DataFlow::PathNode source, DataFlow::PathNode sink |
63+
cfg.hasFlowPath(source, sink) and
64+
cfg.isSource(source.getNode(), authCodeURLCall)
65+
)
66+
}
67+
68+
/**
69+
* Holds if the provided CallNode is within the same root as a call
70+
* to a scanner that reads from os.Stdin.
71+
*/
72+
predicate rootContainsCallToStdinScanner(DataFlow::CallNode authCodeURLCall) {
73+
exists(Fmt::ScannerCall scannerCall | scannerCall.getRoot() = authCodeURLCall.getRoot())
74+
or
75+
exists(Fmt::FScannerCall fScannerCall |
76+
fScannerCall.getReader() = any(ValueEntity v | v.hasQualifiedName("os", "Stdin")).getARead() and
77+
fScannerCall.getRoot() = authCodeURLCall.getRoot()
78+
)
79+
}
80+
81+
from
82+
ConstantStateFlowConf cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
83+
DataFlow::CallNode sinkCall
84+
where
85+
cfg.hasFlowPath(source, sink) and
86+
cfg.isSink(sink.getNode(), sinkCall) and
87+
// Exclude cases that seem to be oauth flows done from within a terminal:
88+
not (
89+
flowsToPrinter(sinkCall) and
90+
rootContainsCallToStdinScanner(sinkCall)
91+
)
4792
select sink.getNode(), source, sink, "Using a constant $@ to create oauth2 URLs.", source.getNode(),
4893
"state string"

‎ql/src/semmle/go/frameworks/Stdlib.qll‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ module Fmt {
9494
}
9595

9696
/** The `Print` function or one of its variants. */
97-
private class Printer extends Function {
97+
class Printer extends Function {
9898
Printer() { this.hasQualifiedName("fmt", ["Print", "Printf", "Println"]) }
9999
}
100100

@@ -131,6 +131,31 @@ module Fmt {
131131
exists(int i | if getName() = "Sscanf" then i > 1 else i > 0 | output.isParameter(i))
132132
}
133133
}
134+
135+
/** The `Scan` function or one of its variants, all of which read from os.Stdin */
136+
class Scanner extends Function {
137+
Scanner() { this.hasQualifiedName("fmt", ["Scan", "Scanf", "Scanln"]) }
138+
}
139+
140+
/** A call to a `Scanner`. */
141+
class ScannerCall extends DataFlow::CallNode {
142+
ScannerCall() { this.getTarget() instanceof Scanner }
143+
}
144+
145+
/**
146+
* The `Fscan` function or one of its variants,
147+
* all of which read from a specified io.Reader
148+
*/
149+
class FScanner extends Function {
150+
FScanner() { this.hasQualifiedName("fmt", ["Fscan", "Fscanf", "Fscanln"]) }
151+
}
152+
153+
/** A call to a `FScanner`. */
154+
class FScannerCall extends DataFlow::CallNode {
155+
FScannerCall() { this.getTarget() instanceof FScanner }
156+
157+
DataFlow::Node getReader() { result = this.getArgument(0) }
158+
}
134159
}
135160

136161
/** Provides models of commonly used functions in the `io` package. */

‎ql/test/experimental/CWE-352/ConstantOauth2State.go‎

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ package main
55
import (
66
"crypto/rand"
77
"encoding/base64"
8+
"fmt"
9+
"log"
810
"net/http"
11+
"os"
912

1013
"golang.org/x/oauth2"
1114
)
@@ -93,7 +96,7 @@ func betterWithVariableStateReturned(w http.ResponseWriter) {
9396
}
9497

9598
state := generateStateOauthCookie(w)
96-
url := conf.AuthCodeURL(state) // GOOD
99+
url := conf.AuthCodeURL(state) // OK, because the state is not a constant.
97100
_ = url
98101
// ...
99102
}
@@ -104,3 +107,86 @@ func generateStateOauthCookie(w http.ResponseWriter) string {
104107
// TODO: save the state string to cookies or HTML storage.
105108
return state
106109
}
110+
func okWithMixedVarState(w http.ResponseWriter) {
111+
conf := &oauth2.Config{
112+
ClientID: "YOUR_CLIENT_ID",
113+
ClientSecret: "YOUR_CLIENT_SECRET",
114+
Scopes: []string{"SCOPE1", "SCOPE2"},
115+
Endpoint: oauth2.Endpoint{
116+
AuthURL: "https://provider.com/o/oauth2/auth",
117+
TokenURL: "https://provider.com/o/oauth2/token",
118+
},
119+
}
120+
121+
state := fmt.Sprintf("%s-%s", stateStringVar, NewCSRFToken())
122+
123+
url := conf.AuthCodeURL(state) // OK, because the state is not a constant.
124+
_ = url
125+
// ...
126+
}
127+
128+
func NewCSRFToken() string {
129+
b := make([]byte, 128)
130+
rand.Read(b)
131+
randomToken := base64.URLEncoding.EncodeToString(b)
132+
return randomToken
133+
}
134+
func okWithConstStatePrinter(w http.ResponseWriter) {
135+
conf := &oauth2.Config{
136+
ClientID: "YOUR_CLIENT_ID",
137+
ClientSecret: "YOUR_CLIENT_SECRET",
138+
Scopes: []string{"SCOPE1", "SCOPE2"},
139+
Endpoint: oauth2.Endpoint{
140+
AuthURL: "https://provider.com/o/oauth2/auth",
141+
TokenURL: "https://provider.com/o/oauth2/token",
142+
},
143+
}
144+
145+
url := conf.AuthCodeURL(stateStringConst) // OK, because we're supposedly not exposed to the web, but within a terminal.
146+
fmt.Printf("Visit the URL for the auth dialog: %v", url)
147+
// ...
148+
149+
var code string
150+
if _, err := fmt.Scan(&code); err != nil {
151+
log.Fatal(err)
152+
}
153+
_ = code
154+
// ...
155+
}
156+
func okWithConstStateFPrinter(w http.ResponseWriter) {
157+
conf := &oauth2.Config{
158+
ClientID: "YOUR_CLIENT_ID",
159+
ClientSecret: "YOUR_CLIENT_SECRET",
160+
Scopes: []string{"SCOPE1", "SCOPE2"},
161+
Endpoint: oauth2.Endpoint{
162+
AuthURL: "https://provider.com/o/oauth2/auth",
163+
TokenURL: "https://provider.com/o/oauth2/token",
164+
},
165+
}
166+
167+
url := conf.AuthCodeURL(stateStringConst) // OK, because we're supposedly not exposed to the web, but within a terminal.
168+
fmt.Printf("Visit the URL for the auth dialog: %v", url)
169+
// ...
170+
171+
var code string
172+
if _, err := fmt.Fscan(os.Stdin, &code); err != nil {
173+
log.Fatal(err)
174+
}
175+
_ = code
176+
// ...
177+
}
178+
func badWithConstStatePrinter(w http.ResponseWriter) {
179+
conf := &oauth2.Config{
180+
ClientID: "YOUR_CLIENT_ID",
181+
ClientSecret: "YOUR_CLIENT_SECRET",
182+
Scopes: []string{"SCOPE1", "SCOPE2"},
183+
Endpoint: oauth2.Endpoint{
184+
AuthURL: "https://provider.com/o/oauth2/auth",
185+
TokenURL: "https://provider.com/o/oauth2/token",
186+
},
187+
}
188+
189+
url := conf.AuthCodeURL(stateStringConst) // BAD
190+
fmt.Printf("LOG: URL %v", url)
191+
// ...
192+
}

0 commit comments

Comments
 (0)