Skip to content

Commit f52d32a

Browse files
erik-krogherik-krogh
committed
add a js/samesite-none-cookie cookie
1 parent bb786bc commit f52d32a

File tree

9 files changed

+291
-1
lines changed

9 files changed

+291
-1
lines changed

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

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ module CookieWrites {
2727
*/
2828
abstract predicate isSensitive();
2929

30+
/**
31+
* Gets the SameSite attribute of the cookie if present.
32+
* Either "Strict", "Lax" or "None".
33+
*/
34+
abstract string getSameSite();
35+
3036
/**
3137
* Holds if the cookie write happens on a server, i.e. the `httpOnly` flag is relevant.
3238
*/
@@ -95,6 +101,14 @@ private predicate hasCookieAttribute(string s, string attribute) {
95101
s.regexpMatch("(?i).*;\\s*" + attribute + "\\b\\s*;?.*$")
96102
}
97103

104+
/**
105+
* Gets the value for a `Set-Cookie` header attribute.
106+
*/
107+
bindingset[s, attribute]
108+
private string getCookieValue(string s, string attribute) {
109+
result = s.regexpCapture("(?i).*;\\s*" + attribute + "=(\\w+)\\b\\s*;?.*$", 1)
110+
}
111+
98112
/**
99113
* A model of the `js-cookie` library (https://github.com/js-cookie/js-cookie).
100114
*/
@@ -105,7 +119,9 @@ private module JsCookie {
105119
DataFlow::CallNode libMemberCall(string name) {
106120
result = DataFlow::globalVarRef("Cookie").getAMemberCall(name) or
107121
result = DataFlow::globalVarRef("Cookie").getAMemberCall("noConflict").getAMemberCall(name) or
108-
result = DataFlow::moduleMember("js-cookie", name).getACall()
122+
result = DataFlow::moduleMember("js-cookie", name).getACall() or
123+
// es-cookie behaves basically the same as js-cookie
124+
result = DataFlow::moduleMember("es-cookie", name).getACall()
109125
}
110126

111127
class ReadAccess extends PersistentReadAccess, DataFlow::CallNode {
@@ -132,6 +148,10 @@ private module JsCookie {
132148
}
133149

134150
override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
151+
152+
override string getSameSite() {
153+
this.getOptionArgument(2, "sameSite").mayHaveStringValue(result)
154+
}
135155
}
136156
}
137157

@@ -173,6 +193,15 @@ private module BrowserCookies {
173193
}
174194

175195
override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
196+
197+
override string getSameSite() {
198+
this.getOptionArgument(2, "samesite").mayHaveStringValue(result)
199+
or
200+
// or, an explicit default has been set
201+
DataFlow::moduleMember("browser-cookies", "defaults")
202+
.getAPropertyWrite("samesite")
203+
.mayHaveStringValue(result)
204+
}
176205
}
177206
}
178207

@@ -211,6 +240,13 @@ private module LibCookie {
211240
}
212241

213242
override predicate isSensitive() { canHaveSensitiveCookie(this.getArgument(0)) }
243+
244+
override string getSameSite() {
245+
this.getOptionArgument(2, "sameSite").mayHaveStringValue(result)
246+
or
247+
this.getOptionArgument(2, "sameSite").mayHaveBooleanValue(true) and
248+
result = "Strict"
249+
}
214250
}
215251
}
216252

@@ -242,6 +278,13 @@ private module ExpressCookies {
242278
not value.mayHaveBooleanValue(false) // anything but `false` is accepted as being maybe true
243279
)
244280
}
281+
282+
override string getSameSite() {
283+
this.getOptionArgument(2, "sameSite").mayHaveStringValue(result)
284+
or
285+
this.getOptionArgument(2, "sameSite").mayHaveBooleanValue(true) and
286+
result = "Strict"
287+
}
245288
}
246289

247290
/**
@@ -268,6 +311,13 @@ private module ExpressCookies {
268311
// A cookie is httpOnly if the `httpOnly` flag is not explicitly set to `false`.
269312
not this.getCookieFlagValue(CookieWrites::httpOnly()).mayHaveBooleanValue(false)
270313
}
314+
315+
override string getSameSite() {
316+
this.getCookieFlagValue("sameSite").mayHaveStringValue(result)
317+
or
318+
this.getCookieFlagValue("sameSite").mayHaveBooleanValue(true) and
319+
result = "Strict"
320+
}
271321
}
272322

273323
/**
@@ -297,6 +347,13 @@ private module ExpressCookies {
297347
// A cookie is httpOnly if the `httpOnly` flag is not explicitly set to `false`.
298348
not this.getCookieFlagValue(CookieWrites::httpOnly()).mayHaveBooleanValue(false)
299349
}
350+
351+
override string getSameSite() {
352+
this.getCookieFlagValue("sameSite").mayHaveStringValue(result)
353+
or
354+
this.getCookieFlagValue("sameSite").mayHaveBooleanValue(true) and
355+
result = "Strict"
356+
}
300357
}
301358
}
302359

@@ -339,6 +396,8 @@ private class HTTPCookieWrite extends CookieWrites::CookieWrite {
339396
}
340397

341398
override predicate isSensitive() { canHaveSensitiveCookie(this) }
399+
400+
override string getSameSite() { result = getCookieValue(header, "SameSite") }
342401
}
343402

344403
/**
@@ -365,4 +424,6 @@ private class DocumentCookieWrite extends CookieWrites::ClientSideCookieWrite {
365424
}
366425

367426
override predicate isSensitive() { canHaveSensitiveCookie(write.getRhs()) }
427+
428+
override string getSameSite() { result = getCookieValue(cookie, "SameSite") }
368429
}

‎javascript/ql/src/Security/CWE-1275/SameSiteNoneCookie.qhelp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Authentication cookies where the SameSite attribute is set to "None" can
9+
potentially be used to perform cross-site request forgery (CSRF) attacks
10+
if no other CSRF protections are in place.
11+
</p>
12+
<p>
13+
With SameSite set to "None" a third party website may create an authorized cross-site request
14+
that includes the cookie.
15+
Such a cross-site request can allow that website to perform actions on behalf of a user.
16+
</p>
17+
</overview>
18+
19+
<recommendation>
20+
<p>
21+
Set the <code>SameSite</code> attribute to <code>Strict</code> on all sensitive cookies.
22+
</p>
23+
</recommendation>
24+
25+
<example>
26+
<p>
27+
The following example stores an authentication token in a cookie where the <code>SameSite</code>
28+
attribute is set to <code>None</code>.
29+
</p>
30+
<sample src="examples/SameSiteCookieBad.js"/>
31+
<p>
32+
To prevent the cookie from being included in cross-site requests, set the <code>SameSite</code>
33+
attribute to <code>Strict</code>.
34+
</p>
35+
<sample src="examples/SameSiteCookieGood.js"/>
36+
</example>
37+
38+
<references>
39+
<li>MDN Web Docs: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite">SameSite cookies</a>.</li>
40+
<li>OWASP: <a href="https://owasp.org/www-community/SameSite">SameSite</a>.</li>
41+
</references>
42+
43+
</qhelp>

‎javascript/ql/src/Security/CWE-1275/SameSiteNoneCookie.ql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Sensitive cookie without SameSite restrictions
3+
* @description Sensitive cookies where the SameSite attribute is set to "None" can
4+
* in some cases allow for Cross-site request forgery (CSRF) attacks.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @security-severity 5.0
8+
* @precision medium
9+
* @id js/samesite-none-cookie
10+
* @tags security
11+
* external/cwe/cwe-1275
12+
*/
13+
14+
import javascript
15+
16+
from CookieWrites::CookieWrite cookie
17+
where
18+
cookie.isSensitive() and
19+
cookie.isSecure() and // `js/clear-text-cookie` will report it if the cookie is not secure.
20+
cookie.getSameSite().toLowerCase() = "none"
21+
select cookie, "Sensitive cookie with SameSite set to 'None'"

‎javascript/ql/src/Security/CWE-1275/examples/SameSiteCookieBad.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const http = require('http');
2+
3+
const server = http.createServer((req, res) => {
4+
res.setHeader("Set-Cookie", `authKey=${makeAuthkey()}; secure; httpOnly; SameSite=None`);
5+
res.writeHead(200, { 'Content-Type': 'text/html' });
6+
res.end('<h2>Hello world</h2>');
7+
});

‎javascript/ql/src/Security/CWE-1275/examples/SameSiteCookieGood.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const http = require('http');
2+
3+
const server = http.createServer((req, res) => {
4+
res.setHeader("Set-Cookie", `authKey=${makeAuthkey()}; secure; httpOnly; SameSite=Strict`);
5+
res.writeHead(200, { 'Content-Type': 'text/html' });
6+
res.end('<h2>Hello world</h2>');
7+
});

‎javascript/ql/src/change-notes/2022-01-24-samesite-cookie.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* A new query `js/samesite-none-cookie` has been added. The query detects when the SameSite attribute on a sensitive cookie is set to None.

‎javascript/ql/test/query-tests/Security/CWE-1275/SameSiteNoneCookie.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
| tst-sameSite.js:4:3:8:4 | Cookies ... OK\\n }) | Sensitive cookie with SameSite set to 'None' |
2+
| tst-sameSite.js:20:3:25:4 | cookies ... OK\\n }) | Sensitive cookie with SameSite set to 'None' |
3+
| tst-sameSite.js:38:19:43:4 | cookie. ... ",\\n }) | Sensitive cookie with SameSite set to 'None' |
4+
| tst-sameSite.js:58:3:63:4 | res.coo ... OK\\n }) | Sensitive cookie with SameSite set to 'None' |
5+
| tst-sameSite.js:76:3:82:4 | session ... OK\\n }) | Sensitive cookie with SameSite set to 'None' |
6+
| tst-sameSite.js:98:3:106:4 | express ... },\\n }) | Sensitive cookie with SameSite set to 'None' |
7+
| tst-sameSite.js:126:33:126:70 | "authKe ... Secure" | Sensitive cookie with SameSite set to 'None' |
8+
| tst-sameSite.js:134:3:134:17 | document.cookie | Sensitive cookie with SameSite set to 'None' |

‎javascript/ql/test/query-tests/Security/CWE-1275/SameSiteNoneCookie.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-1275/SameSiteNoneCookie.ql

‎javascript/ql/test/query-tests/Security/CWE-1275/tst-sameSite.js

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import * as Cookies from "es-cookie";
2+
3+
function esCookies() {
4+
Cookies.set("authkey", "value", {
5+
secure: true,
6+
httpOnly: true,
7+
sameSite: "None", // NOT OK
8+
});
9+
10+
Cookies.set("authkey", "value", {
11+
secure: true,
12+
httpOnly: true,
13+
sameSite: "Strict", // OK
14+
});
15+
}
16+
17+
function browserCookies() {
18+
var cookies = require("browser-cookies");
19+
20+
cookies.set("authkey", "value", {
21+
expires: 365,
22+
secure: true,
23+
httponly: true,
24+
samesite: "None", // NOT OK
25+
});
26+
27+
cookies.set("authkey", "value", {
28+
expires: 365,
29+
secure: true,
30+
httponly: true,
31+
samesite: "Strict", // OK
32+
});
33+
}
34+
35+
function cookie() {
36+
var cookie = require("cookie");
37+
38+
var setCookie = cookie.serialize("authkey", "value", {
39+
maxAge: 9000000000,
40+
httpOnly: true,
41+
secure: true,
42+
sameSite: "None",
43+
});
44+
45+
var setCookie = cookie.serialize("authkey", "value", {
46+
maxAge: 9000000000,
47+
httpOnly: true,
48+
secure: true,
49+
sameSite: true, // OK
50+
});
51+
}
52+
53+
const express = require("express");
54+
const app = express();
55+
const session = require("cookie-session");
56+
57+
app.get("/a", function (req, res, next) {
58+
res.cookie("authkey", "value", {
59+
maxAge: 9000000000,
60+
httpOnly: true,
61+
secure: true,
62+
sameSite: "None", // NOT OK
63+
});
64+
65+
res.cookie("session", "value", {
66+
maxAge: 9000000000,
67+
httpOnly: true,
68+
secure: true,
69+
sameSite: "Strict", // OK
70+
});
71+
72+
res.end("ok");
73+
});
74+
75+
app.use(
76+
session({
77+
name: "session",
78+
keys: ["key1", "key2"],
79+
httpOnly: true,
80+
secure: true,
81+
sameSite: "None", // NOT OK
82+
})
83+
);
84+
85+
app.use(
86+
session({
87+
name: "session",
88+
keys: ["key1", "key2"],
89+
httpOnly: true,
90+
secure: true,
91+
sameSite: "Strict", // OK
92+
})
93+
);
94+
95+
const expressSession = require("express-session");
96+
97+
app.use(
98+
expressSession({
99+
name: "session",
100+
keys: ["key1", "key2"],
101+
cookie: {
102+
httpOnly: true,
103+
secure: true,
104+
sameSite: "None", // NOT OK
105+
},
106+
})
107+
);
108+
109+
app.use(
110+
expressSession({
111+
name: "session",
112+
keys: ["key1", "key2"],
113+
cookie: {
114+
httpOnly: true,
115+
secure: true,
116+
sameSite: "Strict", // OK
117+
},
118+
})
119+
);
120+
121+
const http = require("http");
122+
123+
function test1() {
124+
const server = http.createServer((req, res) => {
125+
res.setHeader("Content-Type", "text/html");
126+
res.setHeader("Set-Cookie", "authKey=ninja; SameSite=None; Secure"); // NOT OK
127+
res.setHeader("Set-Cookie", "authKey=ninja; SameSite=Strict; Secure"); // OK
128+
res.writeHead(200, { "Content-Type": "text/plain" });
129+
res.end("ok");
130+
});
131+
}
132+
133+
function documentCookie() {
134+
document.cookie = "authKey=ninja; SameSite=None; Secure"; // NOT OK
135+
document.cookie = "authKey=ninja; SameSite=Strict; Secure"; // OK
136+
}
137+
138+
// TODO: Add good examples.

0 commit comments

Comments
 (0)