-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Go: Promote non-httponly cookie query, and add insecure cookie query #20762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
joefarebrother
wants to merge
17
commits into
github:main
Choose a base branch
from
joefarebrother:go-insecure-cookie
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,548
−1,156
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
79f1818
Implement cookie write concepts and httponly query
joefarebrother 5c1db90
Fixes, add secure query
joefarebrother 9ce5975
Add modeling for gin
joefarebrother 533b633
Add tests
joefarebrother c7a8712
Fixes and doc updates
joefarebrother 87bf748
Add qhelp & fix tests
joefarebrother c4e9b10
Remove experimental query
joefarebrother 1c96aff
Update integration tests
joefarebrother 999eff1
Add change note
joefarebrother 4c18577
Update integration test
joefarebrother 837aba8
Update formatting
joefarebrother a078a4e
Fix typos
joefarebrother 7decc2b
Address reviews - rename and update doc comments
joefarebrother e2f0fa8
Refactor parts of SensitiveCookieNameConfig
joefarebrother ca68c89
Split SecureCookies into query specific files
joefarebrother 05c294e
Address reviews - update comments, remove unneeded stubs
joefarebrother e7b8eed
Remove references to gorilla
joefarebrother File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /** | ||
| * Provides classes for modeling the `github.com/gin-gonic/gin` package. | ||
| */ | ||
|
|
||
| import go | ||
| import semmle.go.concepts.HTTP | ||
|
|
||
| /** Provides models for the `gin-gonic/gin` package. */ | ||
| module Gin { | ||
| /** Gets the package name `github.com/gin-gonic/gin`. */ | ||
| string packagePath() { result = package("github.com/gin-gonic/gin", "") } | ||
|
|
||
| private class GinCookieWrite extends Http::CookieWrite::Range, DataFlow::MethodCallNode { | ||
| GinCookieWrite() { this.getTarget().hasQualifiedName(packagePath(), "Context", "SetCookie") } | ||
|
|
||
| override DataFlow::Node getName() { result = this.getArgument(0) } | ||
|
|
||
| override DataFlow::Node getValue() { result = this.getArgument(1) } | ||
|
|
||
| override DataFlow::Node getSecure() { result = this.getArgument(5) } | ||
|
|
||
| override DataFlow::Node getHttpOnly() { result = this.getArgument(6) } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| /** Provides classes and predicates for identifying HTTP cookies without the `HttpOnly` attribute. */ | ||
|
|
||
| import go | ||
| import semmle.go.concepts.HTTP | ||
| import semmle.go.dataflow.DataFlow | ||
|
|
||
| private module SensitiveCookieNameConfig implements DataFlow::ConfigSig { | ||
| /** | ||
| * Holds if `source` is an expression with a name or literal value `val` indicating a sensitive cookie. | ||
| */ | ||
| additional predicate isSource(DataFlow::Node source, string val) { | ||
| ( | ||
| val = source.asExpr().getStringValue() or | ||
| val = source.asExpr().(Name).getTarget().getName() | ||
| ) and | ||
| val.regexpMatch("(?i).*(session|login|token|user|auth|credential).*") and | ||
| not val.regexpMatch("(?i).*(xsrf|csrf|forgery).*") | ||
| } | ||
|
|
||
| predicate isSource(DataFlow::Node source) { isSource(source, _) } | ||
|
|
||
| additional predicate isSink(DataFlow::Node sink, Http::CookieWrite cw) { sink = cw.getName() } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { isSink(sink, _) } | ||
|
|
||
| predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { | ||
| exists(Http::CookieOptionWrite co | co.getName() = pred and co.getCookieOutput() = succ) | ||
| } | ||
| } | ||
|
|
||
| /** Tracks flow from sensitive names to HTTP cookie writes. */ | ||
| module SensitiveCookieNameFlow = TaintTracking::Global<SensitiveCookieNameConfig>; | ||
|
|
||
| private module BooleanCookieHttpOnlyConfig implements DataFlow::ConfigSig { | ||
| predicate isSource(DataFlow::Node source) { | ||
| source.getType().getUnderlyingType() instanceof BoolType | ||
| } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getHttpOnly()) } | ||
|
|
||
| predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { | ||
| exists(Http::CookieOptionWrite co | co.getHttpOnly() = pred and co.getCookieOutput() = succ) | ||
| } | ||
| } | ||
|
|
||
| /** Tracks flow from boolean expressions to the `HttpOnly` attribute of HTTP cookie writes. */ | ||
| module BooleanCookieHttpOnlyFlow = TaintTracking::Global<BooleanCookieHttpOnlyConfig>; | ||
|
|
||
| /** Holds if `cw` has the `HttpOnly` attribute left at its default value of `false`. */ | ||
| predicate isNonHttpOnlyDefault(Http::CookieWrite cw) { | ||
| not BooleanCookieHttpOnlyFlow::flowTo(cw.getHttpOnly()) | ||
| } | ||
|
|
||
| /** Holds if `cw` has the `HttpOnly` attribute explicitly set to `false`, from the expression `boolFalse`. */ | ||
| predicate isNonHttpOnlyDirect(Http::CookieWrite cw, Expr boolFalse) { | ||
| BooleanCookieHttpOnlyFlow::flow(DataFlow::exprNode(boolFalse), cw.getHttpOnly()) and | ||
| boolFalse.getBoolValue() = false | ||
| } | ||
|
|
||
| /** Holds if `cw` has the `HttpOnly` attribute set to `false`, either explicitly or by default. */ | ||
| predicate isNonHttpOnlyCookie(Http::CookieWrite cw) { | ||
| isNonHttpOnlyDefault(cw) or | ||
| isNonHttpOnlyDirect(cw, _) | ||
| } | ||
|
|
||
| /** | ||
| * Holds if `cw` has the sensitive name `name`, from the expression `nameExpr`. | ||
| * `source` and `sink` represent the data flow path from the sensitive name expression to the cookie write. | ||
| */ | ||
| predicate isSensitiveCookie( | ||
| Http::CookieWrite cw, string name, SensitiveCookieNameFlow::PathNode source, | ||
| SensitiveCookieNameFlow::PathNode sink | ||
| ) { | ||
| SensitiveCookieNameFlow::flowPath(source, sink) and | ||
| SensitiveCookieNameConfig::isSource(source.getNode(), name) and | ||
| SensitiveCookieNameConfig::isSink(sink.getNode(), cw) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /** Provides classes and predicates for identifying HTTP cookies without the `Secure` attribute. */ | ||
|
|
||
| import go | ||
| import semmle.go.concepts.HTTP | ||
| import semmle.go.dataflow.DataFlow | ||
|
|
||
| private module BooleanCookieSecureConfig implements DataFlow::ConfigSig { | ||
| predicate isSource(DataFlow::Node source) { | ||
| source.getType().getUnderlyingType() instanceof BoolType | ||
| } | ||
|
|
||
| predicate isSink(DataFlow::Node sink) { exists(Http::CookieWrite cw | sink = cw.getSecure()) } | ||
|
|
||
| predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { | ||
| exists(Http::CookieOptionWrite co | co.getSecure() = pred and co.getCookieOutput() = succ) | ||
| } | ||
| } | ||
|
|
||
| /** Tracks flow from boolean expressions to the `Secure` attribute of HTTP cookie writes. */ | ||
| module BooleanCookieSecureFlow = TaintTracking::Global<BooleanCookieSecureConfig>; | ||
|
|
||
| /** Holds if `cw` has the `Secure` attribute left at its default value of `false`. */ | ||
| predicate isInsecureDefault(Http::CookieWrite cw) { | ||
| not BooleanCookieSecureFlow::flowTo(cw.getSecure()) | ||
| } | ||
|
|
||
| /** Holds if `cw` has the `Secure` attribute explicitly set to `false`, from the expression `boolFalse`. */ | ||
| predicate isInsecureDirect(Http::CookieWrite cw, Expr boolFalse) { | ||
| BooleanCookieSecureFlow::flow(DataFlow::exprNode(boolFalse), cw.getSecure()) and | ||
| boolFalse.getBoolValue() = false | ||
| } | ||
|
|
||
| /** Holds if `cw` has the `Secure` attribute set to `false`, either explicitly or by default. */ | ||
| predicate isInsecureCookie(Http::CookieWrite cw) { | ||
| isInsecureDefault(cw) or | ||
| isInsecureDirect(cw, _) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <!DOCTYPE qhelp PUBLIC | ||
| "-//Semmle//qhelp//EN" | ||
| "qhelp.dtd"> | ||
| <qhelp> | ||
|
|
||
| <overview> | ||
| <p>Cookies without the <code>HttpOnly</code> flag set are accessible to client-side scripts such as JavaScript running in the same origin. | ||
| In case of a Cross-Site Scripting (XSS) vulnerability, the cookie can be stolen by a malicious script. | ||
| If a sensitive cookie does not need to be accessed directly by client-side JS, the <code>HttpOnly</code> flag should be set.</p> | ||
| </overview> | ||
|
|
||
| <recommendation> | ||
| <p> | ||
| Set the <code>HttpOnly</code> flag to <code>true</code> for authentication cookies to ensure they are not accessible to client-side scripts. | ||
| </p> | ||
| </recommendation> | ||
|
|
||
| <example> | ||
| <p> | ||
| In the following example, in the case marked BAD, the <code>HttpOnly</code> flag is not set, so the default value of <code>false</code> is used. | ||
| In the case marked GOOD, the <code>HttpOnly</code> flag is set to <code>true</code>. | ||
| </p> | ||
| <sample src="examples/CookieWithoutHttpOnly.go"/> | ||
|
|
||
|
|
||
| </example> | ||
|
|
||
| <references> | ||
|
|
||
| <li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header.</li> | ||
| <li>PortSwigger: <a href="https://portswigger.net/kb/issues/00500600_cookie-without-httponly-flag-set">Cookie without HttpOnly flag set</a></li> | ||
|
|
||
| </references> | ||
| </qhelp> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.