diff --git a/ql/src/experimental/CWE-1004/AuthCookie.qll b/ql/src/experimental/CWE-1004/AuthCookie.qll index 97d86af0360..1a3b04e0b0f 100644 --- a/ql/src/experimental/CWE-1004/AuthCookie.qll +++ b/ql/src/experimental/CWE-1004/AuthCookie.qll @@ -173,6 +173,16 @@ private class GorillaSessionSaveSink extends DataFlow::Node { } } +private class GorillaStoreSaveSink extends DataFlow::Node { + GorillaStoreSaveSink() { + exists(DataFlow::MethodCallNode mcn | + this = mcn.getArgument(2) and + mcn.getTarget() + .hasQualifiedName(package("github.com/gorilla/sessions", ""), "CookieStore", "Save") + ) + } +} + /** * Tracks from gorilla cookie store creation to `gorilla/sessions.Session.Save`. */ @@ -188,7 +198,10 @@ class GorillaCookieStoreSaveTrackingConfiguration extends DataFlow::Configuratio .hasQualifiedName(package("github.com/gorilla/sessions", ""), "NewCookieStore") } - override predicate isSink(DataFlow::Node sink) { sink instanceof GorillaSessionSaveSink } + override predicate isSink(DataFlow::Node sink) { + sink instanceof GorillaSessionSaveSink or + sink instanceof GorillaStoreSaveSink + } override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { exists(DataFlow::MethodCallNode cn | diff --git a/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.expected b/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.expected index eea40a1b2b1..ddefa04709e 100644 --- a/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.expected +++ b/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.expected @@ -76,6 +76,8 @@ edges | CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:146:16:146:20 | store : pointer type | | CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:158:16:158:20 | store : pointer type | | CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:170:16:170:20 | store : pointer type | +| CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:183:16:183:20 | store : pointer type | +| CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:195:16:195:20 | store : pointer type | | CookieWithoutHttpOnly.go:126:16:126:20 | store : pointer type | CookieWithoutHttpOnly.go:129:2:129:8 | session | | CookieWithoutHttpOnly.go:133:14:133:18 | false : bool | CookieWithoutHttpOnly.go:135:2:135:8 | implicit dereference : Session | | CookieWithoutHttpOnly.go:133:14:133:18 | false : bool | CookieWithoutHttpOnly.go:137:2:137:8 | implicit dereference : Session | @@ -156,6 +158,8 @@ edges | CookieWithoutHttpOnly.go:173:21:176:2 | struct literal : Options | CookieWithoutHttpOnly.go:171:2:171:8 | implicit dereference : Session | | CookieWithoutHttpOnly.go:173:21:176:2 | struct literal : Options | CookieWithoutHttpOnly.go:173:2:173:8 | implicit dereference : Session | | CookieWithoutHttpOnly.go:173:21:176:2 | struct literal : Options | CookieWithoutHttpOnly.go:178:2:178:8 | session | +| CookieWithoutHttpOnly.go:183:16:183:20 | store : pointer type | CookieWithoutHttpOnly.go:191:19:191:25 | session | +| CookieWithoutHttpOnly.go:195:16:195:20 | store : pointer type | CookieWithoutHttpOnly.go:202:19:202:25 | session | nodes | CookieWithoutHttpOnly.go:11:7:14:2 | struct literal : Cookie | semmle.label | struct literal : Cookie | | CookieWithoutHttpOnly.go:15:20:15:21 | &... | semmle.label | &... | @@ -257,7 +261,11 @@ nodes | CookieWithoutHttpOnly.go:173:21:176:2 | struct literal : Options | semmle.label | struct literal : Options | | CookieWithoutHttpOnly.go:178:2:178:8 | session | semmle.label | session | | CookieWithoutHttpOnly.go:178:2:178:8 | session | semmle.label | session | -| CookieWithoutHttpOnly.go:190:66:190:70 | false | semmle.label | false | +| CookieWithoutHttpOnly.go:183:16:183:20 | store : pointer type | semmle.label | store : pointer type | +| CookieWithoutHttpOnly.go:191:19:191:25 | session | semmle.label | session | +| CookieWithoutHttpOnly.go:195:16:195:20 | store : pointer type | semmle.label | store : pointer type | +| CookieWithoutHttpOnly.go:202:19:202:25 | session | semmle.label | session | +| CookieWithoutHttpOnly.go:214:66:214:70 | false | semmle.label | false | #select | CookieWithoutHttpOnly.go:15:20:15:21 | &... | CookieWithoutHttpOnly.go:11:7:14:2 | struct literal : Cookie | CookieWithoutHttpOnly.go:15:20:15:21 | &... | Cookie attribute 'HttpOnly' is not set to true. | | CookieWithoutHttpOnly.go:24:20:24:21 | &... | CookieWithoutHttpOnly.go:22:13:22:17 | false : bool | CookieWithoutHttpOnly.go:24:20:24:21 | &... | Cookie attribute 'HttpOnly' is not set to true. | @@ -269,4 +277,6 @@ nodes | CookieWithoutHttpOnly.go:129:2:129:8 | session | CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:129:2:129:8 | session | Cookie attribute 'HttpOnly' is not set to true. | | CookieWithoutHttpOnly.go:142:2:142:8 | session | CookieWithoutHttpOnly.go:133:14:133:18 | false : bool | CookieWithoutHttpOnly.go:142:2:142:8 | session | Cookie attribute 'HttpOnly' is not set to true. | | CookieWithoutHttpOnly.go:153:2:153:8 | session | CookieWithoutHttpOnly.go:149:21:151:2 | struct literal : Options | CookieWithoutHttpOnly.go:153:2:153:8 | session | Cookie attribute 'HttpOnly' is not set to true. | -| CookieWithoutHttpOnly.go:190:66:190:70 | false | CookieWithoutHttpOnly.go:190:66:190:70 | false | CookieWithoutHttpOnly.go:190:66:190:70 | false | Cookie attribute 'HttpOnly' is not set to true. | +| CookieWithoutHttpOnly.go:191:19:191:25 | session | CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:191:19:191:25 | session | Cookie attribute 'HttpOnly' is not set to true. | +| CookieWithoutHttpOnly.go:202:19:202:25 | session | CookieWithoutHttpOnly.go:123:13:123:49 | call to NewCookieStore : pointer type | CookieWithoutHttpOnly.go:202:19:202:25 | session | Cookie attribute 'HttpOnly' is not set to true. | +| CookieWithoutHttpOnly.go:214:66:214:70 | false | CookieWithoutHttpOnly.go:214:66:214:70 | false | CookieWithoutHttpOnly.go:214:66:214:70 | false | Cookie attribute 'HttpOnly' is not set to true. | diff --git a/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.go b/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.go index 6908878e7b3..00be0bbf44a 100644 --- a/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.go +++ b/ql/test/experimental/CWE-1004/CookieWithoutHttpOnly.go @@ -178,6 +178,30 @@ func handler17(w http.ResponseWriter, r *http.Request, httpOnly bool) { session.Save(r, w) // GOOD: value is unknown } +func handler18(w http.ResponseWriter, r *http.Request) { + httpOnly := false + session, _ := store.Get(r, "session-name") + session.Values["foo"] = "secret" + + session.Options = &sessions.Options{ + MaxAge: -1, + HttpOnly: httpOnly, + } + + store.Save(r, w, session) // BAD: Explicitly set to false +} + +func handler19(w http.ResponseWriter, r *http.Request) { + session, _ := store.Get(r, "session-name") + session.Values["foo"] = "secret" + + session.Options = &sessions.Options{ + MaxAge: -1, + } + + store.Save(r, w, session) // BAD: default (false) is used +} + func main() { router := gin.Default()