Skip to content

Commit ced4843

Browse files
committed
change CookieDefinition to a DataFlow::Node
1 parent 24b8455 commit ced4843

File tree

5 files changed

+21
-24
lines changed

5 files changed

+21
-24
lines changed

javascript/ql/lib/semmle/javascript/frameworks/CookieLibraries.qll

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -271,30 +271,27 @@ private module ExpressCookies {
271271
/**
272272
* A cookie set using `response.cookie` from `express` module (https://expressjs.com/en/api.html#res.cookie).
273273
*/
274-
private class InsecureExpressCookieResponse extends CookieWrites::CookieWrite,
275-
DataFlow::MethodCallNode {
276-
InsecureExpressCookieResponse() { this.asExpr() instanceof Express::SetCookie }
277-
274+
private class InsecureExpressCookieResponse extends CookieWrites::CookieWrite instanceof Express::SetCookie {
278275
override predicate isSecure() {
279276
// A cookie is secure if there are cookie options with the `secure` flag set to `true`.
280277
// The default is `false`.
281-
exists(DataFlow::Node value | value = this.getOptionArgument(2, CookieWrites::secure()) |
278+
exists(DataFlow::Node value | value = super.getOptionArgument(2, CookieWrites::secure()) |
282279
not value.mayHaveBooleanValue(false) // anything but `false` is accepted as being maybe true
283280
)
284281
}
285282

286-
override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
283+
override predicate isSensitive() { canHaveSensitiveCookie(super.getArgument(0)) }
287284

288285
override predicate isHttpOnly() {
289286
// A cookie is httpOnly if there are cookie options with the `httpOnly` flag set to `true`.
290287
// The default is `false`.
291-
exists(DataFlow::Node value | value = this.getOptionArgument(2, CookieWrites::httpOnly()) |
288+
exists(DataFlow::Node value | value = super.getOptionArgument(2, CookieWrites::httpOnly()) |
292289
not value.mayHaveBooleanValue(false) // anything but `false` is accepted as being maybe true
293290
)
294291
}
295292

296293
override string getSameSite() {
297-
result = getSameSiteValue(this.getOptionArgument(2, "sameSite"))
294+
result = getSameSiteValue(super.getOptionArgument(2, "sameSite"))
298295
}
299296
}
300297

@@ -372,10 +369,10 @@ private class HttpCookieWrite extends CookieWrites::CookieWrite {
372369

373370
HttpCookieWrite() {
374371
exists(HTTP::CookieDefinition setCookie |
375-
this.asExpr() = setCookie.getHeaderArgument() and
372+
this = setCookie.getHeaderArgument() and
376373
not this instanceof DataFlow::ArrayCreationNode
377374
or
378-
this = setCookie.getHeaderArgument().flow().(DataFlow::ArrayCreationNode).getAnElement()
375+
this = setCookie.getHeaderArgument().(DataFlow::ArrayCreationNode).getAnElement()
379376
) and
380377
header =
381378
[

javascript/ql/lib/semmle/javascript/frameworks/Express.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,14 +774,14 @@ module Express {
774774
/**
775775
* An invocation of the `cookie` method on an HTTP response object.
776776
*/
777-
class SetCookie extends HTTP::CookieDefinition, MethodCallExpr {
777+
class SetCookie extends HTTP::CookieDefinition, DataFlow::MethodCallNode {
778778
ResponseSource response;
779779

780-
SetCookie() { this = response.ref().getAMethodCall("cookie").asExpr() }
780+
SetCookie() { this = response.ref().getAMethodCall("cookie") }
781781

782-
override Expr getNameArgument() { result = this.getArgument(0) }
782+
override DataFlow::Node getNameArgument() { result = this.getArgument(0) }
783783

784-
override Expr getValueArgument() { result = this.getArgument(1) }
784+
override DataFlow::Node getValueArgument() { result = this.getArgument(1) }
785785

786786
override RouteHandler getRouteHandler() { result = response.getRouteHandler() }
787787
}

javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,22 +133,21 @@ module HTTP {
133133
/**
134134
* An expression that sets a cookie in an HTTP response.
135135
*/
136-
abstract class CookieDefinition extends Expr {
137-
// TODO: DataFlow::Node
136+
abstract class CookieDefinition extends DataFlow::Node {
138137
/**
139138
* Gets the argument, if any, specifying the raw cookie header.
140139
*/
141-
Expr getHeaderArgument() { none() } // TODO: DataFlow::Node
140+
DataFlow::Node getHeaderArgument() { none() }
142141

143142
/**
144143
* Gets the argument, if any, specifying the cookie name.
145144
*/
146-
Expr getNameArgument() { none() } // TODO: DataFlow::Node
145+
DataFlow::Node getNameArgument() { none() }
147146

148147
/**
149148
* Gets the argument, if any, specifying the cookie value.
150149
*/
151-
Expr getValueArgument() { none() } // TODO: DataFlow::Node
150+
DataFlow::Node getValueArgument() { none() }
152151

153152
/** Gets the route handler that sets this cookie. */
154153
abstract RouteHandler getRouteHandler();
@@ -161,12 +160,12 @@ module HTTP {
161160
HeaderDefinition header;
162161

163162
SetCookieHeader() {
164-
this = header.asExpr() and
163+
this = header and
165164
header.getAHeaderName() = "set-cookie"
166165
}
167166

168-
override Expr getHeaderArgument() {
169-
header.(ExplicitHeaderDefinition).definesHeaderValue("set-cookie", result.flow())
167+
override DataFlow::Node getHeaderArgument() {
168+
header.(ExplicitHeaderDefinition).definesHeaderValue("set-cookie", result)
170169
}
171170

172171
override RouteHandler getRouteHandler() { result = header.getRouteHandler() }

javascript/ql/lib/semmle/javascript/security/dataflow/CleartextStorageCustomizations.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ module CleartextStorage {
5252
class CookieStorageSink extends Sink {
5353
CookieStorageSink() {
5454
exists(HTTP::CookieDefinition cookieDef |
55-
this.asExpr() = cookieDef.getValueArgument() or
56-
this.asExpr() = cookieDef.getHeaderArgument()
55+
this = cookieDef.getValueArgument() or
56+
this = cookieDef.getHeaderArgument()
5757
)
5858
}
5959
}

javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class DomPropWriteNode extends Assignment {
144144
* A value written to web storage, like `localStorage` or `sessionStorage`.
145145
*/
146146
class WebStorageWrite extends Expr {
147+
// TODO: DataFlow::Node
147148
WebStorageWrite() {
148149
exists(DataFlow::SourceNode webStorage |
149150
webStorage = DataFlow::globalVarRef("localStorage") or

0 commit comments

Comments
 (0)