11/**
22 * @name Sensitive cookies without the HttpOnly response header set
3- * @description Sensitive cookies without the 'HttpOnly' flag set leaves session cookies vulnerable to
3+ * @description A sensitive cookie without the 'HttpOnly' flag set may be vulnerable to
44 * an XSS attack.
55 * @kind path-problem
66 * @problem.severity warning
7- * @precision medium
7+ * @precision high
8+ * @security-severity 5.0
89 * @id java/sensitive-cookie-not-httponly
910 * @tags security
10- * experimental
1111 * external/cwe/cwe-1004
1212 */
1313
1414/*
1515 * Sketch of the structure of this query: we track cookie names that appear to be sensitive
1616 * (e.g. `session` or `token`) to a `ServletResponse.addHeader(...)` or `.addCookie(...)`
1717 * method that does not set the `httpOnly` flag. Subsidiary configurations
18- * `MatchesHttpOnlyConfiguration ` and `SetHttpOnlyInCookieConfiguration ` are used to establish
18+ * `MatchesHttpOnlyToRawHeaderConfig ` and `SetHttpOnlyInCookieConfig ` are used to establish
1919 * when the `httpOnly` flag is likely to have been set, before configuration
20- * `MissingHttpOnlyConfiguration ` establishes that a non-`httpOnly` cookie has a sensitive-seeming name.
20+ * `MissingHttpOnlyConfig ` establishes that a non-`httpOnly` cookie has a sensitive-seeming name.
2121 */
2222
2323import java
2424import semmle.code.java.dataflow.FlowSteps
2525import semmle.code.java.frameworks.Servlets
2626import semmle.code.java.dataflow.TaintTracking
27- import MissingHttpOnlyFlow:: PathGraph
2827
2928/** Gets a regular expression for matching common names of sensitive cookies. */
3029string getSensitiveCookieNameRegex ( ) { result = "(?i).*(auth|session|token|key|credential).*" }
@@ -50,8 +49,8 @@ class SensitiveCookieNameExpr extends Expr {
5049}
5150
5251/** A method call that sets a `Set-Cookie` header. */
53- class SetCookieMethodCall extends MethodCall {
54- SetCookieMethodCall ( ) {
52+ class SetCookieRawHeaderMethodCall extends MethodCall {
53+ SetCookieRawHeaderMethodCall ( ) {
5554 (
5655 this .getMethod ( ) instanceof ResponseAddHeaderMethod or
5756 this .getMethod ( ) instanceof ResponseSetHeaderMethod
@@ -62,19 +61,19 @@ class SetCookieMethodCall extends MethodCall {
6261
6362/**
6463 * A taint configuration tracking flow from the text `httponly` to argument 1 of
65- * `SetCookieMethodCall `.
64+ * `SetCookieRawHeaderMethodCall `.
6665 */
67- module MatchesHttpOnlyConfig implements DataFlow:: ConfigSig {
66+ module MatchesHttpOnlyToRawHeaderConfig implements DataFlow:: ConfigSig {
6867 predicate isSource ( DataFlow:: Node source ) {
6968 source .asExpr ( ) .( CompileTimeConstantExpr ) .getStringValue ( ) .toLowerCase ( ) .matches ( "%httponly%" )
7069 }
7170
7271 predicate isSink ( DataFlow:: Node sink ) {
73- sink .asExpr ( ) = any ( SetCookieMethodCall ma ) .getArgument ( 1 )
72+ sink .asExpr ( ) = any ( SetCookieRawHeaderMethodCall ma ) .getArgument ( 1 )
7473 }
7574}
7675
77- module MatchesHttpOnlyFlow = TaintTracking:: Global< MatchesHttpOnlyConfig > ;
76+ module MatchesHttpOnlyToRawHeaderFlow = TaintTracking:: Global< MatchesHttpOnlyToRawHeaderConfig > ;
7877
7978/** A class descended from `javax.servlet.http.Cookie`. */
8079class CookieClass extends RefType {
@@ -103,29 +102,11 @@ predicate removesCookie(MethodCall ma) {
103102}
104103
105104/**
106- * Holds if the MethodCall `ma` is a test method call indicated by:
107- * a) in a test directory such as `src/test/java`
108- * b) in a test package whose name has the word `test`
109- * c) in a test class whose name has the word `test`
110- * d) in a test class implementing a test framework such as JUnit or TestNG
105+ * A taint configuration tracking the flow of a cookie that has had the
106+ * `HttpOnly` flag set, or has been removed, to a `ServletResponse.addCookie`
107+ * call.
111108 */
112- predicate isTestMethod ( MethodCall ma ) {
113- exists ( Method m |
114- m = ma .getEnclosingCallable ( ) and
115- (
116- m .getDeclaringType ( ) .getName ( ) .toLowerCase ( ) .matches ( "%test%" ) or // Simple check to exclude test classes to reduce FPs
117- m .getDeclaringType ( ) .getPackage ( ) .getName ( ) .toLowerCase ( ) .matches ( "%test%" ) or // Simple check to exclude classes in test packages to reduce FPs
118- exists ( m .getLocation ( ) .getFile ( ) .getAbsolutePath ( ) .indexOf ( "/src/test/java" ) ) or // Match test directory structure of build tools like maven
119- m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG
120- )
121- )
122- }
123-
124- /**
125- * A taint configuration tracking flow of a method that sets the `HttpOnly` flag,
126- * or one that removes a cookie, to a `ServletResponse.addCookie` call.
127- */
128- module SetHttpOnlyOrRemovesCookieConfig implements DataFlow:: ConfigSig {
109+ module SetHttpOnlyOrRemovesCookieToAddCookieConfig implements DataFlow:: ConfigSig {
129110 predicate isSource ( DataFlow:: Node source ) {
130111 source .asExpr ( ) =
131112 any ( MethodCall ma | setsCookieHttpOnly ( ma ) or removesCookie ( ma ) ) .getQualifier ( )
@@ -137,25 +118,25 @@ module SetHttpOnlyOrRemovesCookieConfig implements DataFlow::ConfigSig {
137118 }
138119}
139120
140- module SetHttpOnlyOrRemovesCookieFlow = TaintTracking:: Global< SetHttpOnlyOrRemovesCookieConfig > ;
121+ module SetHttpOnlyOrRemovesCookieToAddCookieFlow =
122+ TaintTracking:: Global< SetHttpOnlyOrRemovesCookieToAddCookieConfig > ;
141123
142124/**
143- * A cookie that is added to an HTTP response and which doesn't have `httpOnly ` set, used as a sink
144- * in `MissingHttpOnlyConfiguration `.
125+ * A cookie that is added to an HTTP response and which doesn't have `HttpOnly ` set, used as a sink
126+ * in `MissingHttpOnlyConfig `.
145127 */
146- class CookieResponseSink extends DataFlow:: ExprNode {
147- CookieResponseSink ( ) {
128+ class CookieResponseWithoutHttpOnlySink extends DataFlow:: ExprNode {
129+ CookieResponseWithoutHttpOnlySink ( ) {
148130 exists ( MethodCall ma |
149131 (
150132 ma .getMethod ( ) instanceof ResponseAddCookieMethod and
151133 this .getExpr ( ) = ma .getArgument ( 0 ) and
152- not SetHttpOnlyOrRemovesCookieFlow :: flowTo ( this )
134+ not SetHttpOnlyOrRemovesCookieToAddCookieFlow :: flowTo ( this )
153135 or
154- ma instanceof SetCookieMethodCall and
136+ ma instanceof SetCookieRawHeaderMethodCall and
155137 this .getExpr ( ) = ma .getArgument ( 1 ) and
156- not MatchesHttpOnlyFlow:: flowTo ( this ) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure")
157- ) and
158- not isTestMethod ( ma ) // Test class or method
138+ not MatchesHttpOnlyToRawHeaderFlow:: flowTo ( this ) // response.addHeader("Set-Cookie", "token=" +authId + ";HttpOnly;Secure")
139+ )
159140 )
160141 }
161142}
@@ -178,15 +159,21 @@ predicate setsHttpOnlyInNewCookie(ClassInstanceExpr cie) {
178159
179160/**
180161 * A taint configuration tracking flow from a sensitive cookie without the `HttpOnly` flag
181- * set to its HTTP response.
162+ * set to an HTTP response.
163+ *
164+ * Tracks string literals containing sensitive names (`SensitiveCookieNameExpr`), to an `addCookie` call (as a `Cookie` object)
165+ * or an `addHeader` call (as a string) (`CookieResponseWithoutHttpOnlySink`).
166+ *
167+ * Passes through `Cookie` constructors and `toString` calls.
182168 */
183169module MissingHttpOnlyConfig implements DataFlow:: ConfigSig {
184170 predicate isSource ( DataFlow:: Node source ) { source .asExpr ( ) instanceof SensitiveCookieNameExpr }
185171
186- predicate isSink ( DataFlow:: Node sink ) { sink instanceof CookieResponseSink }
172+ predicate isSink ( DataFlow:: Node sink ) { sink instanceof CookieResponseWithoutHttpOnlySink }
187173
188174 predicate isBarrier ( DataFlow:: Node node ) {
189175 // JAX-RS's `new NewCookie("session-access-key", accessKey, "/", null, null, 0, true, true)` and similar
176+ // Cookie constructors that set the `HttpOnly` flag are considered barriers to the flow of sensitive names.
190177 setsHttpOnlyInNewCookie ( node .asExpr ( ) )
191178 }
192179
@@ -212,13 +199,8 @@ module MissingHttpOnlyConfig implements DataFlow::ConfigSig {
212199
213200module MissingHttpOnlyFlow = TaintTracking:: Global< MissingHttpOnlyConfig > ;
214201
215- deprecated query predicate problems (
216- DataFlow:: Node sinkNode , MissingHttpOnlyFlow:: PathNode source , MissingHttpOnlyFlow:: PathNode sink ,
217- string message1 , DataFlow:: Node sourceNode , string message2
218- ) {
219- MissingHttpOnlyFlow:: flowPath ( source , sink ) and
220- sinkNode = sink .getNode ( ) and
221- message1 = "$@ doesn't have the HttpOnly flag set." and
222- sourceNode = source .getNode ( ) and
223- message2 = "This sensitive cookie"
224- }
202+ import MissingHttpOnlyFlow:: PathGraph
203+
204+ from MissingHttpOnlyFlow:: PathNode source , MissingHttpOnlyFlow:: PathNode sink
205+ where MissingHttpOnlyFlow:: flowPath ( source , sink )
206+ select sink , source , sink , "$@ doesn't have the HttpOnly flag set." , source , "This sensitive cookie"
0 commit comments