Skip to content

Commit 7d76619

Browse files
joefarebrotherjoefarebrother
committed
Implement cookie write concepts and httponly query
1 parent 1c2d8bb commit 7d76619

File tree

4 files changed

+233
-0
lines changed

4 files changed

+233
-0
lines changed

‎go/ql/lib/semmle/go/concepts/HTTP.qll‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,4 +380,96 @@ module Http {
380380
/** Gets a node that is used in a check that is tested before this handler is run. */
381381
predicate guardedBy(DataFlow::Node check) { super.guardedBy(check) }
382382
}
383+
384+
/** Provides a class for modelling HTTP response cookie writes. */
385+
module CookieWrite {
386+
/**
387+
* An write of an HTTP Cookie to an HTTP response.
388+
*
389+
* Extend this class to model new APIs. If you want to refine existing API models,
390+
* extend `HTTP::CookieWrite` instead.
391+
*/
392+
abstract class Range extends DataFlow::Node {
393+
/** Gets the name of the cookie written. */
394+
abstract DataFlow::Node getName();
395+
396+
/** Gets the value of the cookie written. */
397+
abstract DataFlow::Node getValue();
398+
399+
/** Gets the `Secure` attribute of the cookie written. */
400+
abstract DataFlow::Node getSecure();
401+
402+
/** Gets the `HttpOnly` attribute of the cookie written. */
403+
abstract DataFlow::Node getHttpOnly();
404+
}
405+
}
406+
407+
/**
408+
* An write of an HTTP Cookie to an HTTP response.
409+
*
410+
* Extend this class to refine existing API models. If you want to model new APIs,
411+
* extend `HTTP::CookieWrite::Range` instead.
412+
*/
413+
class CookieWrite extends DataFlow::Node instanceof CookieWrite::Range {
414+
/** Gets the name of the cookie written. */
415+
DataFlow::Node getName() { result = super.getName() }
416+
417+
/** Gets the value of the cookie written. */
418+
DataFlow::Node getValue() { result = super.getValue() }
419+
420+
/** Gets the `Secure` attribute of the cookie written. */
421+
DataFlow::Node getSecure() { result = super.getSecure() }
422+
423+
/** Gets the `HttpOnly` attribute of the cookie written. */
424+
DataFlow::Node getHttpOnly() { result = super.getHttpOnly() }
425+
}
426+
427+
/** Provides a class for modelling the options of an HTTP cookie. */
428+
module CookieOptions {
429+
/**
430+
* An HTTP Cookie object.
431+
*
432+
* Extend this class to model new APIs. If you want to refine existing API models,
433+
* extend `HTTP::CookieOptions` instead.
434+
*/
435+
abstract class Range extends DataFlow::Node {
436+
/** Gets the node representing the cookie object for the options being set. */
437+
abstract DataFlow::Node getCookieOutput();
438+
439+
/** Gets the name of the cookie represented. */
440+
abstract DataFlow::Node getName();
441+
442+
/** Gets the value of the cookie represented. */
443+
abstract DataFlow::Node getValue();
444+
445+
/** Gets the `Secure` attribute of the cookie represented. */
446+
abstract DataFlow::Node getSecure();
447+
448+
/** Gets the `HttpOnly` attribute of the cookie represented. */
449+
abstract DataFlow::Node getHttpOnly();
450+
}
451+
}
452+
453+
/**
454+
* An HTTP Cookie.
455+
*
456+
* Extend this class to refine existing API models. If you want to model new APIs,
457+
* extend `HTTP::CookieOptions::Range` instead.
458+
*/
459+
class CookieOptions extends DataFlow::Node instanceof CookieOptions::Range {
460+
/** Gets the node representing the cookie object for the options being set. */
461+
DataFlow::Node getCookieOutput() { result = super.getCookieOutput() }
462+
463+
/** Gets the name of the cookie represented. */
464+
DataFlow::Node getName() { result = super.getName() }
465+
466+
/** Gets the value of the cookie represented. */
467+
DataFlow::Node getValue() { result = super.getValue() }
468+
469+
/** Gets the `Secure` attribute of the cookie represented. */
470+
DataFlow::Node getSecure() { result = super.getSecure() }
471+
472+
/** Gets the `HttpOnly` attribute of the cookie represented. */
473+
DataFlow::Node getHttpOnly() { result = super.getHttpOnly() }
474+
}
383475
}

‎go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,4 +293,38 @@ module NetHttp {
293293

294294
override DataFlow::Node getAPathArgument() { result = this.getArgument(2) }
295295
}
296+
297+
class CookieWrite extends Http::CookieWrite::Range, DataFlow::CallNode {
298+
CookieWrite() { this.getTarget().hasQualifiedName(package("net/http", ""), "SetCookie") }
299+
300+
override DataFlow::Node getName() { result = this.getArgument(1) }
301+
302+
override DataFlow::Node getValue() { result = this.getArgument(1) }
303+
304+
override DataFlow::Node getSecure() { result = this.getArgument(1) }
305+
306+
override DataFlow::Node getHttpOnly() { result = this.getArgument(1) }
307+
}
308+
309+
class CookieFieldWrite extends Http::CookieOptions::Range {
310+
Write w;
311+
Field f;
312+
DataFlow::Node written;
313+
string fieldName;
314+
315+
CookieFieldWrite() {
316+
f.hasQualifiedName(package("net/http", ""), "Cookie", fieldName) and
317+
w.writesField(this, f, written)
318+
}
319+
320+
override DataFlow::Node getCookieOutput() { result = this }
321+
322+
override DataFlow::Node getName() { fieldName = "Name" and result = written }
323+
324+
override DataFlow::Node getValue() { fieldName = "Value" and result = written }
325+
326+
override DataFlow::Node getSecure() { fieldName = "Secure" and result = written }
327+
328+
override DataFlow::Node getHttpOnly() { fieldName = "HttpOnly" and result = written }
329+
}
296330
}

‎go/ql/lib/semmle/go/security/SecureCookies.qll‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/** Provides classes and predicates for identifying HTTP cookies with insecure attributes. */
2+
3+
import go
4+
import semmle.go.concepts.HTTP
5+
import semmle.go.dataflow.DataFlow
6+
7+
/**
8+
* Holds if the expression or its value has a sensitive name
9+
*/
10+
private predicate isSensitiveExpr(Expr expr, string val) {
11+
(
12+
val = expr.getStringValue() or
13+
val = expr.(Name).getTarget().getName()
14+
) and
15+
val.regexpMatch("(?i).*(session|login|token|user|auth|credential).*") and
16+
not val.regexpMatch("(?i).*(xsrf|csrf|forgery).*")
17+
}
18+
19+
private module SensitiveCookieNameConfig implements DataFlow::ConfigSig {
20+
predicate isSource(DataFlow::Node source) { isSensitiveExpr(source.asExpr(), _) }
21+
22+
predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getName()) }
23+
24+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
25+
exists(Http::CookieOptions co | co.getName() = pred and co.getCookieOutput() = succ)
26+
}
27+
}
28+
29+
/** Tracks flow from sensitive names to HTTP cookie writes. */
30+
module SensitiveCookieNameFlow = DataFlow::Global<SensitiveCookieNameConfig>;
31+
32+
private module BooleanCookieSecureConfig implements DataFlow::ConfigSig {
33+
predicate isSource(DataFlow::Node source) { exists(source.asExpr().getBoolValue()) }
34+
35+
predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getSecure()) }
36+
37+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
38+
exists(Http::CookieOptions co | co.getSecure() = pred and co.getCookieOutput() = succ)
39+
}
40+
}
41+
42+
/** Tracks flow from boolean expressions to the `Secure` attribute HTTP cookie writes. */
43+
module BooleanCookieSecureFlow = DataFlow::Global<BooleanCookieSecureConfig>;
44+
45+
private module BooleanCookieHttpOnlyConfig implements DataFlow::ConfigSig {
46+
predicate isSource(DataFlow::Node source) { exists(source.asExpr().getBoolValue()) }
47+
48+
predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getHttpOnly()) }
49+
50+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
51+
exists(Http::CookieOptions co | co.getHttpOnly() = pred and co.getCookieOutput() = succ)
52+
}
53+
}
54+
55+
/** Tracks flow from boolean expressions to the `HttpOnly` attribute HTTP cookie writes. */
56+
module BooleanCookieHttpOnlyFlow = DataFlow::Global<BooleanCookieHttpOnlyConfig>;
57+
58+
predicate isInsecureDefault(Http::CookieWrite cw) {
59+
not BooleanCookieSecureFlow::flow(_, cw.getSecure())
60+
}
61+
62+
predicate isNonHttpOnlyDefault(Http::CookieWrite cw) {
63+
not BooleanCookieHttpOnlyFlow::flow(_, cw.getHttpOnly())
64+
}
65+
66+
predicate isInsecureDirect(Http::CookieWrite cw, Expr boolFalse) {
67+
BooleanCookieSecureFlow::flow(DataFlow::exprNode(boolFalse), cw.getSecure()) and
68+
boolFalse.getBoolValue() = false
69+
}
70+
71+
predicate isNonHttpOnlyDirect(Http::CookieWrite cw, Expr boolFalse) {
72+
BooleanCookieHttpOnlyFlow::flow(DataFlow::exprNode(boolFalse), cw.getHttpOnly()) and
73+
boolFalse.getBoolValue() = false
74+
}
75+
76+
predicate isSensitiveCookie(Http::CookieWrite cw, Expr nameExpr, string name) {
77+
SensitiveCookieNameFlow::flow(DataFlow::exprNode(nameExpr), cw.getName()) and
78+
isSensitiveExpr(nameExpr, name)
79+
}

‎go/ql/src/Security/CWE-1004/CookieWithoutHttpOnly.ql‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* @name 'HttpOnly' attribute is not set to true
3+
* @description Omitting the 'HttpOnly' attribute for security sensitive data allows
4+
* malicious JavaScript to steal it in case of XSS vulnerability. Always set
5+
* 'HttpOnly' to 'true' to authentication related cookie to make it
6+
* not accessible by JavaScript.
7+
* @kind problem
8+
* @problem.severity warning
9+
* @precision high
10+
* @id go/cookie-httponly-not-set
11+
* @tags security
12+
* experimental
13+
* external/cwe/cwe-1004
14+
*/
15+
16+
import go
17+
import semmle.go.security.SecureCookies
18+
import semmle.go.concepts.HTTP
19+
20+
from Http::CookieWrite cw, Expr sensitiveNameExpr, string name
21+
where
22+
isSensitiveCookie(cw, sensitiveNameExpr, name) and
23+
(
24+
isNonHttpOnlyDefault(cw)
25+
or
26+
isNonHttpOnlyDirect(cw, _)
27+
)
28+
select cw, "Sensitive cookie $@ does not set HttpOnly to true", sensitiveNameExpr, name

0 commit comments

Comments
 (0)