Skip to content

Commit 2654aff

Browse files
geoffw0geoffw0
committed
Rust: Account for the 'secure' and 'partitioned' attributes.
1 parent 257a1b0 commit 2654aff

File tree

8 files changed

+213
-134
lines changed

8 files changed

+213
-134
lines changed

‎rust/ql/lib/codeql/rust/frameworks/biscotti.model.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ extensions:
1616
pack: codeql/rust-all
1717
extensible: summaryModel
1818
data:
19-
- ["<biscotti::response_cookie::ResponseCookie>::set_secure", "Argument[self]", "ReturnValue", "taint", "manual"]
20-
- ["<biscotti::response_cookie::ResponseCookie>::set_partitioned", "Argument[self]", "ReturnValue", "taint", "manual"]
19+
- ["<biscotti::response_cookie::ResponseCookie>::set_secure", "Argument[self].OptionalBarrier[cookie-secure-arg0]", "ReturnValue", "taint", "manual"]
20+
- ["<biscotti::response_cookie::ResponseCookie>::set_partitioned", "Argument[self].OptionalBarrier[cookie-partitioned-arg0]", "ReturnValue", "taint", "manual"]
2121
- ["<biscotti::response_cookie::ResponseCookie>::set_name", "Argument[self]", "ReturnValue", "taint", "manual"]
2222
- ["<biscotti::response_cookie::ResponseCookie>::set_value", "Argument[self]", "ReturnValue", "taint", "manual"]
2323
- ["<biscotti::response_cookie::ResponseCookie>::set_http_only", "Argument[self]", "ReturnValue", "taint", "manual"]

‎rust/ql/lib/codeql/rust/frameworks/cookie.model.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ extensions:
2626
pack: codeql/rust-all
2727
extensible: summaryModel
2828
data:
29-
- ["<cookie::builder::CookieBuilder>::secure", "Argument[self]", "ReturnValue", "taint", "manual"]
30-
- ["<cookie::builder::CookieBuilder>::partitioned", "Argument[self]", "ReturnValue", "taint", "manual"]
29+
- ["<cookie::builder::CookieBuilder>::secure", "Argument[self].OptionalBarrier[cookie-secure-arg0]", "ReturnValue", "taint", "manual"]
30+
- ["<cookie::builder::CookieBuilder>::partitioned", "Argument[self].OptionalBarrier[cookie-partitioned-arg0]", "ReturnValue", "taint", "manual"]
3131
- ["<cookie::builder::CookieBuilder>::expires", "Argument[self]", "ReturnValue", "taint", "manual"]
3232
- ["<cookie::builder::CookieBuilder>::max_age", "Argument[self]", "ReturnValue", "taint", "manual"]
3333
- ["<cookie::builder::CookieBuilder>::domain", "Argument[self]", "ReturnValue", "taint", "manual"]
@@ -36,5 +36,5 @@ extensions:
3636
- ["<cookie::builder::CookieBuilder>::same_site", "Argument[self]", "ReturnValue", "taint", "manual"]
3737
- ["<cookie::builder::CookieBuilder>::permanent", "Argument[self]", "ReturnValue", "taint", "manual"]
3838
- ["<cookie::builder::CookieBuilder>::removal", "Argument[self]", "ReturnValue", "taint", "manual"]
39-
- ["<cookie::Cookie>::set_secure", "Argument[self]", "ReturnValue", "taint", "manual"]
40-
- ["<cookie::Cookie>::set_partitioned", "Argument[self]", "ReturnValue", "taint", "manual"]
39+
- ["<cookie::Cookie>::set_secure", "Argument[self].OptionalBarrier[cookie-secure-arg0]", "ReturnValue", "taint", "manual"]
40+
- ["<cookie::Cookie>::set_partitioned", "Argument[self].OptionalBarrier[cookie-partitioned-arg0]", "ReturnValue", "taint", "manual"]

‎rust/ql/lib/codeql/rust/security/InsecureCookieExtensions.qll‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,36 @@ module InsecureCookie {
4646
private class ModelsAsDataSink extends Sink {
4747
ModelsAsDataSink() { sinkNode(this, "cookie-use") }
4848
}
49+
50+
/**
51+
* Holds if cookie attribute `attrib` (`secure` or `partitioned`) is set to `value` (`true` or `false`) at `node`.
52+
* A value that cannot be determined is treated as `false`.
53+
*
54+
* This references models-as-data optional barrier nodes, for example `OptionalBarrier[cookie-secure-arg0]`.
55+
*/
56+
predicate cookieSetNode(DataFlow::Node node, string attrib, boolean value) {
57+
exists(
58+
FlowSummaryNode summaryNode, string barrierName, CallExprBase ce, int arg,
59+
DataFlow::Node argNode
60+
|
61+
// decode a `cookie-`... optional barrier
62+
DataflowImpl::optionalBarrier(summaryNode, barrierName) and
63+
attrib = barrierName.regexpCapture("cookie-(secure|partitioned)-arg([0-9]+)", 1) and
64+
arg = barrierName.regexpCapture("cookie-(secure|partitioned)-arg([0-9]+)", 2).toInt() and
65+
// find a call and arg referenced by this optional barrier
66+
ce.getStaticTarget() = summaryNode.getSummarizedCallable() and
67+
ce.getArg(arg) = argNode.asExpr().getExpr() and
68+
// check if the argument is always `true`
69+
(
70+
if
71+
forex(DataFlow::Node argSourceNode | DataFlow::localFlow(argSourceNode, argNode) |
72+
argSourceNode.asExpr().getExpr().(BooleanLiteralExpr).getTextValue() = "true"
73+
)
74+
then value = true // `true` flow to here
75+
else value = false // `false` or unknown
76+
) and
77+
// and the node `node` where this happens
78+
node.asExpr().getExpr() = ce
79+
)
80+
}
4981
}

‎rust/ql/src/queries/security/CWE-614/InsecureCookie.ql‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,18 @@ import codeql.rust.security.InsecureCookieExtensions
2020

2121
/**
2222
* A data flow configuration for tracking values representing cookies without the
23-
* 'secure' attribute set.
23+
* 'secure' attribute set. This is the primary data flow configurationn for this
24+
* query.
2425
*/
2526
module InsecureCookieConfig implements DataFlow::ConfigSig {
2627
import InsecureCookie
2728

2829
predicate isSource(DataFlow::Node node) {
2930
// creation of a cookie or cookie configuration with default, insecure settings
3031
node instanceof Source
32+
or
33+
// setting the 'secure' attribute to false (or an unknown value)
34+
cookieSetNode(node, "secure", false)
3135
}
3236

3337
predicate isSink(DataFlow::Node node) {
@@ -36,6 +40,37 @@ module InsecureCookieConfig implements DataFlow::ConfigSig {
3640
}
3741

3842
predicate isBarrier(DataFlow::Node node) {
43+
// setting the 'secure' attribute to true
44+
cookieSetNode(node, "secure", true)
45+
or
46+
node instanceof Barrier
47+
}
48+
49+
predicate observeDiffInformedIncrementalMode() { any() }
50+
}
51+
52+
/**
53+
* A data flow configuration for tracking values representing cookies with the
54+
* 'partitioned' attribute set. This is a secondary data flow configuration used
55+
* to filter out unwanted results.
56+
*/
57+
module PartitionedCookieConfig implements DataFlow::ConfigSig {
58+
import InsecureCookie
59+
60+
predicate isSource(DataFlow::Node node) {
61+
// setting the 'partitioned' attribute to true
62+
cookieSetNode(node, "partitioned", true)
63+
}
64+
65+
predicate isSink(DataFlow::Node node) {
66+
// use of a cookie or cookie configuration
67+
node instanceof Sink
68+
}
69+
70+
predicate isBarrier(DataFlow::Node node) {
71+
// setting the 'partitioned' attribute to false (or an unknown value)
72+
cookieSetNode(node, "partitioned", false)
73+
or
3974
node instanceof Barrier
4075
}
4176

@@ -44,9 +79,12 @@ module InsecureCookieConfig implements DataFlow::ConfigSig {
4479

4580
module InsecureCookieFlow = TaintTracking::Global<InsecureCookieConfig>;
4681

82+
module PartitionedCookieFlow = TaintTracking::Global<PartitionedCookieConfig>;
83+
4784
import InsecureCookieFlow::PathGraph
4885

4986
from InsecureCookieFlow::PathNode sourceNode, InsecureCookieFlow::PathNode sinkNode
5087
where
51-
InsecureCookieFlow::flowPath(sourceNode, sinkNode)
88+
InsecureCookieFlow::flowPath(sourceNode, sinkNode) and
89+
not PartitionedCookieFlow::flow(_, sinkNode.getNode())
5290
select sinkNode.getNode(), sourceNode, sinkNode, "Cookie attribute 'Secure' is not set to true."

‎rust/ql/test/query-tests/security/CWE-614/CookieSet.expected‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
| main.rs:8:19:8:64 | ... .secure(...) | secure | false |
2+
| main.rs:12:19:12:63 | ... .secure(...) | secure | true |
3+
| main.rs:20:5:20:54 | ... .secure(...) | secure | false |
4+
| main.rs:21:5:21:55 | ... .secure(...) | secure | false |
5+
| main.rs:24:5:24:51 | ... .secure(...) | secure | false |
6+
| main.rs:25:5:25:52 | ... .secure(...) | secure | false |
7+
| main.rs:26:5:26:50 | ... .secure(...) | secure | false |
8+
| main.rs:27:5:27:51 | ... .secure(...) | secure | false |
9+
| main.rs:28:5:28:60 | ... .secure(...) | secure | false |
10+
| main.rs:29:5:29:60 | ... .secure(...) | secure | false |
11+
| main.rs:33:9:33:58 | ... .secure(...) | secure | false |
12+
| main.rs:35:9:35:58 | ... .secure(...) | secure | false |
13+
| main.rs:39:5:39:53 | ... .secure(...) | secure | false |
14+
| main.rs:40:5:40:64 | ... .secure(...) | secure | false |
15+
| main.rs:41:5:41:93 | ... .secure(...) | secure | false |
16+
| main.rs:42:5:42:72 | ... .secure(...) | secure | false |
17+
| main.rs:43:5:43:60 | ... .secure(...) | secure | false |
18+
| main.rs:44:5:44:66 | ... .secure(...) | secure | false |
19+
| main.rs:45:5:45:86 | ... .secure(...) | secure | false |
20+
| main.rs:46:5:46:62 | ... .secure(...) | secure | false |
21+
| main.rs:47:5:47:60 | ... .secure(...) | secure | false |
22+
| main.rs:48:5:48:50 | ... .secure(...) | secure | false |
23+
| main.rs:49:5:49:39 | ... .secure(...) | secure | false |
24+
| main.rs:50:5:50:54 | ... .secure(...) | secure | false |
25+
| main.rs:53:5:53:49 | ... .secure(...) | secure | true |
26+
| main.rs:53:5:53:63 | ... .secure(...) | secure | false |
27+
| main.rs:54:5:54:50 | ... .secure(...) | secure | false |
28+
| main.rs:54:5:54:63 | ... .secure(...) | secure | true |
29+
| main.rs:61:5:61:22 | a.set_secure(...) | secure | true |
30+
| main.rs:63:5:63:23 | a.set_secure(...) | secure | false |
31+
| main.rs:71:5:71:27 | b.set_secure(...) | secure | false |
32+
| main.rs:73:5:73:22 | b.set_secure(...) | secure | true |
33+
| main.rs:81:9:81:26 | c.set_secure(...) | secure | true |
34+
| main.rs:84:5:84:22 | c.set_secure(...) | secure | true |
35+
| main.rs:90:9:90:26 | c.set_secure(...) | secure | true |
36+
| main.rs:92:9:92:31 | c.set_partitioned(...) | partitioned | true |
37+
| main.rs:109:9:109:26 | e.set_secure(...) | secure | true |
38+
| main.rs:114:5:114:54 | ... .partitioned(...) | partitioned | true |
39+
| main.rs:126:13:126:30 | a.set_secure(...) | secure | true |
40+
| main.rs:130:13:130:31 | b.set_secure(...) | secure | false |
41+
| main.rs:134:13:134:35 | c.set_partitioned(...) | partitioned | true |
42+
| main.rs:138:13:138:30 | d.set_secure(...) | secure | true |
43+
| main.rs:142:13:142:36 | e.set_partitioned(...) | partitioned | false |
44+
| main.rs:146:13:146:31 | f.set_secure(...) | secure | false |

‎rust/ql/test/query-tests/security/CWE-614/CookieSet.ql‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import rust
2+
import codeql.rust.dataflow.DataFlow
3+
import codeql.rust.security.InsecureCookieExtensions
4+
5+
from DataFlow::Node node, string state, boolean value
6+
where InsecureCookie::cookieSetNode(node, state, value)
7+
select node, state, value

0 commit comments

Comments
 (0)