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

Commit 4206408

Browse files
authored
Merge pull request #153 from max-schaefer/cleanup-107
More cleanup
2 parents 1d910a9 + 223d0db commit 4206408

26 files changed

Lines changed: 74 additions & 75 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Modeling of several WebSocket libraries has been added, which may lead to more results from the
3+
security queries.

ql/src/experimental/CWE-807/SensitiveConditionBypass.qhelp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,26 @@ Testing untrusted user input against a fixed constant results in
66
a bypass of the conditional check as the attacker may alter the input to match the constant.
77
When an incorrect check of this type is used to guard a potentially sensitive block,
88
it results an attacker gaining access to the sensitive block.
9-
</p>
9+
</p>
1010
</overview>
1111
<recommendation>
1212
<p>
1313
Never decide whether to authenticate a user based on data that may be controlled by that user.
1414
If necessary, ensure that the data is validated extensively when it is input before any
1515
authentication checks are performed.
16-
</p>
17-
<p>
16+
</p>
17+
<p>
1818
It is still possible to have a system that "remembers" users, thus not requiring
1919
the user to login on every interaction. For example, personalization settings can be applied
2020
without authentication because this is not sensitive information. However, users
21-
should be allowed to take sensitive actions only when they have been fully authenticated.
21+
should be allowed to take sensitive actions only when they have been fully authenticated.
22+
</p>
2223
</recommendation>
2324
<example>
2425
<p>
2526
The following example shows a comparison where an user controlled
2627
expression is used to guard a sensitive method. This should be avoided.:
27-
</p>
28+
</p>
2829
<sample src="SensitiveConditionBypassBad.go" />
2930
</example>
3031
</qhelp>

ql/src/go.qll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,23 @@ import semmle.go.Scopes
1717
import semmle.go.Stmt
1818
import semmle.go.StringOps
1919
import semmle.go.Types
20+
import semmle.go.Util
2021
import semmle.go.controlflow.BasicBlocks
2122
import semmle.go.controlflow.ControlFlowGraph
2223
import semmle.go.controlflow.IR
2324
import semmle.go.dataflow.DataFlow
2425
import semmle.go.dataflow.GlobalValueNumbering
25-
import semmle.go.dataflow.TaintTracking
2626
import semmle.go.dataflow.SSA
27+
import semmle.go.dataflow.TaintTracking
2728
import semmle.go.frameworks.Email
2829
import semmle.go.frameworks.HTTP
2930
import semmle.go.frameworks.Macaron
3031
import semmle.go.frameworks.Mux
3132
import semmle.go.frameworks.NoSQL
32-
import semmle.go.frameworks.SystemCommandExecutors
3333
import semmle.go.frameworks.SQL
34-
import semmle.go.frameworks.XPath
3534
import semmle.go.frameworks.Stdlib
35+
import semmle.go.frameworks.SystemCommandExecutors
3636
import semmle.go.frameworks.Testing
37-
import semmle.go.frameworks.Websocket
37+
import semmle.go.frameworks.WebSocket
38+
import semmle.go.frameworks.XPath
3839
import semmle.go.security.FlowSources
39-
import semmle.go.Util

ql/src/semmle/go/Packages.qll

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,13 @@ class Package extends @package {
2626
}
2727

2828
/**
29-
* Gets the Go import string that may identify a package in module `mod` with the given path,
30-
* possibly modulo semantic import versioning.
29+
* Gets an import path that identifies a package in module `mod` with the given path,
30+
* possibly modulo [semantic import versioning](https://github.com/golang/go/wiki/Modules#semantic-import-versioning).
31+
*
32+
* For example, `package("github.com/go-pg/pg", "types")` gets an import path that can
33+
* refer to `"github.com/go-pg/pg/types"`, but also to `"github.com/go-pg/pg/v10/types"`.
3134
*/
3235
bindingset[result, mod, path]
3336
string package(string mod, string path) {
34-
result.regexpMatch("\\Q" + mod + "\\E([/.]v[^/]+)?/\\Q" + path + "\\E")
37+
result.regexpMatch("\\Q" + mod + "\\E([/.]v[^/]+)?($|/)\\Q" + path + "\\E")
3538
}

ql/src/semmle/go/dataflow/BarrierGuardUtil.qll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
/**
2-
* Contains implementations of some commonly used barrier
3-
* guards for sanitizing untrusted URLs.
2+
* Provides implementations of some commonly used barrier guards for sanitizing untrusted URLs.
43
*/
54

65
import go

ql/src/semmle/go/frameworks/SQL.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,11 @@ module SQL {
7676

7777
/** A string that might identify package `go-pg/pg` or a specific version of it. */
7878
bindingset[result]
79-
private string gopg() { result.regexpMatch("github.com/go-pg/pg(/v[^/]+)?") }
79+
private string gopg() { result = package("github.com/go-pg/pg", "") }
8080

8181
/** A string that might identify package `go-pg/pg/orm` or a specific version of it. */
8282
bindingset[result]
83-
private string gopgorm() { result.regexpMatch("github.com/go-pg/pg(/v[^/]+)?/orm") }
83+
private string gopgorm() { result = package("github.com/go-pg/pg", "orm") }
8484

8585
/**
8686
* A string argument to an API of `go-pg/pg` that is directly interpreted as SQL without
Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import go
44

55
/**
6-
* A data-flow node that establishes a new WebSocket connection.
6+
* A function call that establishes a new WebSocket connection.
77
*
88
* Extend this class to refine existing API models. If you want to model new APIs,
99
* extend `WebSocketRequestCall::Range` instead.
@@ -20,7 +20,7 @@ class WebSocketRequestCall extends DataFlow::CallNode {
2020
/** Provides classes for working with WebSocket request functions. */
2121
module WebSocketRequestCall {
2222
/**
23-
* A data-flow node that establishes a new WebSocket connection.
23+
* A function call that establishes a new WebSocket connection.
2424
*
2525
* Extend this class to model new APIs. If you want to refine existing
2626
* API models, extend `WebSocketRequestCall` instead.
@@ -31,8 +31,7 @@ module WebSocketRequestCall {
3131
}
3232

3333
/**
34-
* A WebSocket request expression string used in an API function of the
35-
* `golang.org/x/net/websocket` package.
34+
* A call to the `Dial` function of the `golang.org/x/net/websocket` package.
3635
*/
3736
private class GolangXNetDialFunc extends Range {
3837
GolangXNetDialFunc() {
@@ -44,8 +43,7 @@ module WebSocketRequestCall {
4443
}
4544

4645
/**
47-
* A WebSocket DialConfig expression string used in an API function
48-
* of the `golang.org/x/net/websocket` package.
46+
* A call to the `DialConfig` function of the `golang.org/x/net/websocket` package.
4947
*/
5048
private class GolangXNetDialConfigFunc extends Range {
5149
GolangXNetDialConfigFunc() {
@@ -64,13 +62,12 @@ module WebSocketRequestCall {
6462
}
6563

6664
/**
67-
* A WebSocket request expression string used in an API function
68-
* of the `github.com/gorilla/websocket` package.
65+
* A call to the `Dialer` or `DialContext` function of the `github.com/gorilla/websocket` package.
6966
*/
70-
private class GorillaWebsocketDialFunc extends Range {
67+
private class GorillaWebSocketDialFunc extends Range {
7168
DataFlow::Node url;
7269

73-
GorillaWebsocketDialFunc() {
70+
GorillaWebSocketDialFunc() {
7471
// func (d *Dialer) Dial(urlStr string, requestHeader http.Header) (*Conn, *http.Response, error)
7572
// func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader http.Header) (*Conn, *http.Response, error)
7673
exists(string name, Method f |
@@ -87,8 +84,7 @@ module WebSocketRequestCall {
8784
}
8885

8986
/**
90-
* A WebSocket request expression string used in an API function
91-
* of the `github.com/gobwas/ws` package.
87+
* A call to the `Dialer.Dial` method of the `github.com/gobwas/ws` package.
9288
*/
9389
private class GobwasWsDialFunc extends Range {
9490
GobwasWsDialFunc() {
@@ -106,11 +102,10 @@ module WebSocketRequestCall {
106102
}
107103

108104
/**
109-
* A WebSocket request expression string used in an API function
110-
* of the `nhooyr.io/websocket` package.
105+
* A call to the `Dial` function of the `nhooyr.io/websocket` package.
111106
*/
112-
private class NhooyrWebsocketDialFunc extends Range {
113-
NhooyrWebsocketDialFunc() {
107+
private class NhooyrWebSocketDialFunc extends Range {
108+
NhooyrWebSocketDialFunc() {
114109
// func Dial(ctx context.Context, u string, opts *DialOptions) (*Conn, *http.Response, error)
115110
this.getTarget().hasQualifiedName(package("nhooyr.io", "websocket"), "Dial")
116111
}
@@ -119,26 +114,24 @@ module WebSocketRequestCall {
119114
}
120115

121116
/**
122-
* A WebSocket request expression string used in an API function
123-
* of the `github.com/sacOO7/gowebsocket` package.
117+
* A call to the `BuildProxy` or `New` function of the `github.com/sacOO7/gowebsocket` package.
124118
*/
125119
private class SacOO7DialFunc extends Range {
126120
SacOO7DialFunc() {
127121
// func BuildProxy(Url string) func(*http.Request) (*url.URL, error)
128122
// func New(url string) Socket
129-
this.getTarget().hasQualifiedName("github.com/sacOO7/gowebsocket", ["New", "BuildProxy"])
123+
this.getTarget().hasQualifiedName("github.com/sacOO7/gowebsocket", ["BuildProxy", "New"])
130124
}
131125

132126
override DataFlow::Node getRequestUrl() { result = this.getArgument(0) }
133127
}
134128
}
135129

136-
/*
130+
/**
137131
* A message written to a WebSocket, considered as a flow sink for reflected XSS.
138132
*/
139-
140-
class WebsocketReaderAsSource extends UntrustedFlowSource::Range {
141-
WebsocketReaderAsSource() {
133+
class WebSocketReaderAsSource extends UntrustedFlowSource::Range {
134+
WebSocketReaderAsSource() {
142135
exists(WebSocketReader r | this = r.getAnOutput().getNode(r.getACall()))
143136
}
144137
}
@@ -154,7 +147,7 @@ class WebSocketReader extends Function {
154147

155148
WebSocketReader() { this = self }
156149

157-
/** Gets an output of this function that is read from a WebSocket connection. */
150+
/** Gets an output of this function containing data that is read from a WebSocket connection. */
158151
FunctionOutput getAnOutput() { result = self.getAnOutput() }
159152
}
160153

@@ -167,12 +160,12 @@ module WebSocketReader {
167160
* extend `WebSocketReader` instead.
168161
*/
169162
abstract class Range extends Function {
170-
/**Returns the parameter in which the function stores the message read. */
163+
/** Gets an output of this function containing data that is read from a WebSocket connection. */
171164
abstract FunctionOutput getAnOutput();
172165
}
173166

174167
/**
175-
* Models the `Receive` method of the `golang.org/x/net/websocket` package.
168+
* The `Codec.Receive` method of the `golang.org/x/net/websocket` package.
176169
*/
177170
private class GolangXNetCodecRecv extends Range, Method {
178171
GolangXNetCodecRecv() {
@@ -184,7 +177,7 @@ module WebSocketReader {
184177
}
185178

186179
/**
187-
* Models the `Read` method of the `golang.org/x/net/websocket` package.
180+
* The `Conn.Read` method of the `golang.org/x/net/websocket` package.
188181
*/
189182
private class GolangXNetConnRead extends Range, Method {
190183
GolangXNetConnRead() {
@@ -196,10 +189,10 @@ module WebSocketReader {
196189
}
197190

198191
/**
199-
* Models the `Read` method of the `nhooyr.io/websocket` package.
192+
* The `Conn.Read` method of the `nhooyr.io/websocket` package.
200193
*/
201-
private class NhooyrWebsocketRead extends Range, Method {
202-
NhooyrWebsocketRead() {
194+
private class NhooyrWebSocketRead extends Range, Method {
195+
NhooyrWebSocketRead() {
203196
// func (c *Conn) Read(ctx context.Context) (MessageType, []byte, error)
204197
this.hasQualifiedName("nhooyr.io/websocket", "Conn", "Read")
205198
}
@@ -208,10 +201,10 @@ module WebSocketReader {
208201
}
209202

210203
/**
211-
* Models the `Reader` method of the `nhooyr.io/websocket` package.
204+
* The `Conn.Reader` method of the `nhooyr.io/websocket` package.
212205
*/
213-
private class NhooyrWebsocketReader extends Range, Method {
214-
NhooyrWebsocketReader() {
206+
private class NhooyrWebSocketReader extends Range, Method {
207+
NhooyrWebSocketReader() {
215208
// func (c *Conn) Reader(ctx context.Context) (MessageType, io.Reader, error)
216209
this.hasQualifiedName("nhooyr.io/websocket", "Conn", "Reader")
217210
}
@@ -220,7 +213,7 @@ module WebSocketReader {
220213
}
221214

222215
/**
223-
* Models the `ReadFrame`function of the `github.com/gobwas/ws` package.
216+
* The `ReadFrame` function of the `github.com/gobwas/ws` package.
224217
*/
225218
private class GobwasWsReadFrame extends Range {
226219
GobwasWsReadFrame() {
@@ -232,7 +225,7 @@ module WebSocketReader {
232225
}
233226

234227
/**
235-
* Models the `ReadHeader`function of the `github.com/gobwas/ws` package.
228+
* The `ReadHeader` function of the `github.com/gobwas/ws` package.
236229
*/
237230
private class GobwasWsReadHeader extends Range {
238231
GobwasWsReadHeader() {
@@ -244,10 +237,10 @@ module WebSocketReader {
244237
}
245238

246239
/**
247-
* Models the `ReadJson` function of the `github.com/gorilla/websocket` package.
240+
* The `ReadJson` function of the `github.com/gorilla/websocket` package.
248241
*/
249-
private class GorillaWebsocketReadJson extends Range {
250-
GorillaWebsocketReadJson() {
242+
private class GorillaWebSocketReadJson extends Range {
243+
GorillaWebSocketReadJson() {
251244
// func ReadJSON(c *Conn, v interface{}) error
252245
this.hasQualifiedName("github.com/gorilla/websocket", "ReadJSON")
253246
}
@@ -256,10 +249,10 @@ module WebSocketReader {
256249
}
257250

258251
/**
259-
* Models the `ReadJson` method of the `github.com/gorilla/websocket` package.
252+
* The `Conn.ReadJson` method of the `github.com/gorilla/websocket` package.
260253
*/
261-
private class GorillaWebsocketConnReadJson extends Range, Method {
262-
GorillaWebsocketConnReadJson() {
254+
private class GorillaWebSocketConnReadJson extends Range, Method {
255+
GorillaWebSocketConnReadJson() {
263256
// func (c *Conn) ReadJSON(v interface{}) error
264257
this.hasQualifiedName("github.com/gorilla/websocket", "Conn", "ReadJSON")
265258
}
@@ -268,10 +261,10 @@ module WebSocketReader {
268261
}
269262

270263
/**
271-
* Models the `ReadMessage` method of the `github.com/gorilla/websocket` package.
264+
* The `Conn.ReadMessage` method of the `github.com/gorilla/websocket` package.
272265
*/
273-
private class GorillaWebsocketReadMessage extends Range, Method {
274-
GorillaWebsocketReadMessage() {
266+
private class GorillaWebSocketReadMessage extends Range, Method {
267+
GorillaWebSocketReadMessage() {
275268
// func (c *Conn) ReadMessage() (messageType int, p []byte, err error)
276269
this.hasQualifiedName("github.com/gorilla/websocket", "Conn", "ReadMessage")
277270
}

ql/test/library-tests/semmle/go/frameworks/Websocket/DialFunction.expected renamed to ql/test/library-tests/semmle/go/frameworks/WebSocket/DialFunction.expected

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
| DialFunction.go:36:2:36:45 | call to Dial | DialFunction.go:36:31:36:44 | untrustedInput |
88
| DialFunction.go:38:2:38:31 | call to BuildProxy | DialFunction.go:38:17:38:30 | untrustedInput |
99
| DialFunction.go:39:2:39:24 | call to New | DialFunction.go:39:10:39:23 | untrustedInput |
10-
| WebsocketReadWrite.go:30:12:30:42 | call to Dial | WebsocketReadWrite.go:30:27:30:29 | uri |
11-
| WebsocketReadWrite.go:40:14:40:50 | call to Dial | WebsocketReadWrite.go:40:42:40:44 | uri |
12-
| WebsocketReadWrite.go:50:17:50:37 | call to Dial | WebsocketReadWrite.go:50:29:50:31 | uri |
13-
| WebsocketReadWrite.go:66:20:66:51 | call to Dial | WebsocketReadWrite.go:66:48:66:50 | uri |
10+
| WebSocketReadWrite.go:30:12:30:42 | call to Dial | WebSocketReadWrite.go:30:27:30:29 | uri |
11+
| WebSocketReadWrite.go:40:14:40:50 | call to Dial | WebSocketReadWrite.go:40:42:40:44 | uri |
12+
| WebSocketReadWrite.go:50:17:50:37 | call to Dial | WebSocketReadWrite.go:50:29:50:31 | uri |
13+
| WebSocketReadWrite.go:66:20:66:51 | call to Dial | WebSocketReadWrite.go:66:48:66:50 | uri |

ql/test/library-tests/semmle/go/frameworks/Websocket/DialFunction.go renamed to ql/test/library-tests/semmle/go/frameworks/WebSocket/DialFunction.go

File renamed without changes.

ql/test/library-tests/semmle/go/frameworks/Websocket/DialFunction.ql renamed to ql/test/library-tests/semmle/go/frameworks/WebSocket/DialFunction.ql

File renamed without changes.

0 commit comments

Comments
 (0)