Skip to content

Commit 226e4eb

Browse files
joefarebrotherjoefarebrother
committed
Use a 3-valued newtype for hasSameSiteAttribute
1 parent df5569f commit 226e4eb

File tree

8 files changed

+83
-49
lines changed

8 files changed

+83
-49
lines changed

‎python/ql/lib/semmle/python/Concepts.qll

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,9 +1215,9 @@ module Http {
12151215
predicate hasHttpOnlyFlag(boolean b) { super.hasHttpOnlyFlag(b) }
12161216

12171217
/**
1218-
* Holds if the `SameSite` flag of the cookie is known to have a value of `b`.
1218+
* Holds if the `SameSite` attribute of the cookie is known to have a value of `v`.
12191219
*/
1220-
predicate hasSameSiteFlag(boolean b) { super.hasSameSiteFlag(b) }
1220+
predicate hasSameSiteAttribute(CookieWrite::SameSiteValue v) { super.hasSameSiteAttribute(v) }
12211221
}
12221222

12231223
/** Provides a class for modeling new cookie writes on HTTP responses. */
@@ -1288,33 +1288,63 @@ module Http {
12881288
}
12891289

12901290
/**
1291-
* Holds if the `SameSite` flag of the cookie is known to have a value of `b`.
1291+
* Holds if the `SameSite` flag of the cookie is known to have a value of `v`.
12921292
*/
1293-
// TODO: b could be a newtype with 3 values indicating Strict,Lax,or None
1294-
// currently, Strict and Lax are represented with true and None is represented with false.
1295-
predicate hasSameSiteFlag(boolean b) {
1293+
predicate hasSameSiteAttribute(SameSiteValue v) {
12961294
exists(this.getHeaderArg()) and
12971295
(
12981296
exists(StringLiteral sl |
1299-
sl.getText().regexpMatch("(?i).*;\\s*samesite=(strict|lax);.*") and
1297+
sl.getText().regexpMatch("(?i).*;\\s*samesite=strict;.*") and
13001298
TaintTracking::localTaint(DataFlow::exprNode(sl), this.getHeaderArg()) and
1301-
b = true
1299+
v instanceof SameSiteStrict
1300+
)
1301+
or
1302+
exists(StringLiteral sl |
1303+
sl.getText().regexpMatch("(?i).*;\\s*samesite=lax;.*") and
1304+
TaintTracking::localTaint(DataFlow::exprNode(sl), this.getHeaderArg()) and
1305+
v instanceof SameSiteLax
13021306
)
13031307
or
13041308
exists(StringLiteral sl |
13051309
sl.getText().regexpMatch("(?i).*;\\s*samesite=none;.*") and
13061310
TaintTracking::localTaint(DataFlow::exprNode(sl), this.getHeaderArg()) and
1307-
b = false
1311+
v instanceof SameSiteNone
13081312
)
13091313
or
13101314
exists(StringLiteral sl |
13111315
not sl.getText().regexpMatch("(?i).*;\\s*samesite=(strict|lax|none);.*") and
13121316
DataFlow::localFlow(DataFlow::exprNode(sl), this.getHeaderArg()) and
1313-
b = true // Lax is the default
1317+
v instanceof SameSiteLax // Lax is the default
13141318
)
13151319
)
13161320
}
13171321
}
1322+
1323+
private newtype TSameSiteValue =
1324+
TSameSiteStrict() or
1325+
TSameSiteLax() or
1326+
TSameSiteNone()
1327+
1328+
/** A possible value for the SameSite attribute of a cookie. */
1329+
class SameSiteValue extends TSameSiteValue {
1330+
/** Gets a string representation of this value. */
1331+
string toString() { none() }
1332+
}
1333+
1334+
/** A `Strict` value of the `SameSite` attribute. */
1335+
class SameSiteStrict extends SameSiteValue, TSameSiteStrict {
1336+
override string toString() { result = "Strict" }
1337+
}
1338+
1339+
/** A `Lax` value of the `SameSite` attribute. */
1340+
class SameSiteLax extends SameSiteValue, TSameSiteLax {
1341+
override string toString() { result = "Lax" }
1342+
}
1343+
1344+
/** A `None` value of the `SameSite` attribute. */
1345+
class SameSiteNone extends SameSiteValue, TSameSiteNone {
1346+
override string toString() { result = "None" }
1347+
}
13181348
}
13191349

13201350
/** A write to a `Set-Cookie` header that sets a cookie directly. */

‎python/ql/lib/semmle/python/frameworks/Django.qll

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2211,22 +2211,25 @@ module PrivateDjango {
22112211
b = false
22122212
}
22132213

2214-
override predicate hasSameSiteFlag(boolean b) {
2215-
super.hasHttpOnlyFlag(b)
2214+
override predicate hasSameSiteAttribute(Http::Server::CookieWrite::SameSiteValue v) {
2215+
super.hasSameSiteAttribute(v)
22162216
or
22172217
exists(DataFlow::Node arg, StringLiteral str | arg = this.getArgByName("samesite") |
22182218
DataFlow::localFlow(DataFlow::exprNode(str), arg) and
22192219
(
2220-
str.getText().toLowerCase() = ["strict", "lax"] and
2221-
b = true
2220+
str.getText().toLowerCase() = "strict" and
2221+
v instanceof Http::Server::CookieWrite::SameSiteStrict
2222+
or
2223+
str.getText().toLowerCase() = "strict" and
2224+
v instanceof Http::Server::CookieWrite::SameSiteLax
22222225
or
22232226
str.getText().toLowerCase() = "none" and
2224-
b = false
2227+
v instanceof Http::Server::CookieWrite::SameSiteNone
22252228
)
22262229
)
22272230
or
22282231
not exists(this.getArgByName("samesite")) and
2229-
b = true // Lax is the default
2232+
v instanceof Http::Server::CookieWrite::SameSiteLax // Lax is the default
22302233
}
22312234
}
22322235

‎python/ql/lib/semmle/python/frameworks/Flask.qll

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -618,22 +618,25 @@ module Flask {
618618
b = false
619619
}
620620

621-
override predicate hasSameSiteFlag(boolean b) {
622-
super.hasHttpOnlyFlag(b)
621+
override predicate hasSameSiteAttribute(Http::Server::CookieWrite::SameSiteValue v) {
622+
super.hasSameSiteAttribute(v)
623623
or
624624
exists(DataFlow::Node arg, StringLiteral str | arg = this.getArgByName("samesite") |
625625
DataFlow::localFlow(DataFlow::exprNode(str), arg) and
626626
(
627-
str.getText().toLowerCase() = ["strict", "lax"] and
628-
b = true
627+
str.getText().toLowerCase() = "strict" and
628+
v instanceof Http::Server::CookieWrite::SameSiteStrict
629+
or
630+
str.getText().toLowerCase() = "strict" and
631+
v instanceof Http::Server::CookieWrite::SameSiteLax
629632
or
630633
str.getText().toLowerCase() = "none" and
631-
b = false
634+
v instanceof Http::Server::CookieWrite::SameSiteNone
632635
)
633636
)
634637
or
635638
not exists(this.getArgByName("samesite")) and
636-
b = true // Lax is the default
639+
v instanceof Http::Server::CookieWrite::SameSiteLax // Lax is the default
637640
}
638641
}
639642

‎python/ql/src/Security/CWE-614/InsecureCookie.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ predicate hasProblem(Http::Server::CookieWrite cookie, string alert, int idx) {
2626
alert = "HttpOnly" and
2727
idx = 1
2828
or
29-
cookie.hasSameSiteFlag(false) and
29+
cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and
3030
alert = "SameSite" and
3131
idx = 2
3232
}

‎python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ where
2626
cookie.hasHttpOnlyFlag(false) and
2727
alert = "httponly"
2828
or
29-
cookie.hasSameSiteFlag(false) and
29+
cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and
3030
alert = "samesite"
3131
select cookie, "Cookie is added without the '" + alert + "' flag properly set."

‎python/ql/src/experimental/semmle/python/frameworks/Django.qll

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,18 +141,17 @@ private module ExperimentalPrivateDjango {
141141
else b = false
142142
}
143143

144-
override predicate hasSameSiteFlag(boolean b) {
145-
if
146-
exists(StringLiteral str |
147-
str.getText() in ["Strict", "Lax"] and
148-
DataFlow::exprNode(str)
149-
.(DataFlow::LocalSourceNode)
150-
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
151-
)
152-
then b = true
153-
else b = false
154-
}
155-
144+
// override predicate hasSameSiteFlag(boolean b) {
145+
// if
146+
// exists(StringLiteral str |
147+
// str.getText() in ["Strict", "Lax"] and
148+
// DataFlow::exprNode(str)
149+
// .(DataFlow::LocalSourceNode)
150+
// .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
151+
// )
152+
// then b = true
153+
// else b = false
154+
// }
156155
override DataFlow::Node getHeaderArg() { none() }
157156
}
158157
}

‎python/ql/src/experimental/semmle/python/frameworks/Flask.qll

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,17 @@ module ExperimentalFlask {
5656
else b = false
5757
}
5858

59-
override predicate hasSameSiteFlag(boolean b) {
60-
if
61-
exists(StringLiteral str |
62-
str.getText() in ["Strict", "Lax"] and
63-
DataFlow::exprNode(str)
64-
.(DataFlow::LocalSourceNode)
65-
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
66-
)
67-
then b = true
68-
else b = false
69-
}
70-
59+
// override predicate hasSameSiteFlag(boolean b) {
60+
// if
61+
// exists(StringLiteral str |
62+
// str.getText() in ["Strict", "Lax"] and
63+
// DataFlow::exprNode(str)
64+
// .(DataFlow::LocalSourceNode)
65+
// .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
66+
// )
67+
// then b = true
68+
// else b = false
69+
// }
7170
override DataFlow::Node getHeaderArg() { none() }
7271
}
7372
}

‎python/ql/src/experimental/semmle/python/security/injection/CookieInjection.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class CookieSink extends DataFlow::Node {
1818
cookie.hasHttpOnlyFlag(false) and
1919
flag = "httponly"
2020
or
21-
cookie.hasSameSiteFlag(false) and
21+
cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and
2222
flag = "samesite"
2323
)
2424
)

0 commit comments

Comments
 (0)