hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 02:05:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Insecure cookie query: accept ServletRequest.isSecure(), and allow more than one possible input to a setSecure(...) call.

Browse Source
This commit is contained in:
Chris Smowton
2022-05-11 11:59:37 +01:00
parent 1af0e9b619
commit c17ef42cc7
3 changed files with 87 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
java/ql/src/Security/CWE/CWE-614/InsecureCookie.ql
View File

@@ -13,6 +13,18 @@
import java
import semmle.code.java.frameworks.Servlets
import semmle.code.java.dataflow.DataFlow
predicate isSafeSecureCookieSetting(Expr e) {
e.(CompileTimeConstantExpr).getBooleanValue() = true
or
exists(Method isSecure |
isSecure.getName() = "isSecure" and
isSecure.getDeclaringType().getASourceSupertype*() instanceof ServletRequest
|
e.(MethodAccess).getMethod() = isSecure
)
}
from MethodAccess add
where
@@ -20,7 +32,12 @@ where
not exists(Variable cookie, MethodAccess m |
add.getArgument(0) = cookie.getAnAccess() and
m.getMethod().getName() = "setSecure" and
m.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() = true and
forex(DataFlow::Node argSource |
DataFlow::localFlow(argSource, DataFlow::exprNode(m.getArgument(0))) and
not DataFlow::localFlowStep(_, argSource)
|
isSafeSecureCookieSetting(argSource.asExpr())
) and
m.getQualifier() = cookie.getAnAccess()
)
select add, "Cookie is added to response without the 'secure' flag being set."

5
java/ql/test/query-tests/security/CWE-311/CWE-614/semmle/tests/InsecureCookie.expected
View File

@@ -1 +1,4 @@
| Test.java:16:4:16:29 | addCookie(...) | Cookie is added to response without the 'secure' flag being set. |
| Test.java:19:4:19:29 | addCookie(...) | Cookie is added to response without the 'secure' flag being set. |
| Test.java:28:4:28:29 | addCookie(...) | Cookie is added to response without the 'secure' flag being set. |
| Test.java:37:4:37:29 | addCookie(...) | Cookie is added to response without the 'secure' flag being set. |
| Test.java:51:4:51:29 | addCookie(...) | Cookie is added to response without the 'secure' flag being set. |

70
java/ql/test/query-tests/security/CWE-311/CWE-614/semmle/tests/Test.java
View File

@@ -8,21 +8,81 @@ package test.cwe614.semmle.tests;
import javax.servlet.http.*;
class Test {
public static void test(HttpServletRequest request, HttpServletResponse response) {
public static final boolean constTrue = true;
public static void test(HttpServletRequest request, HttpServletResponse response, boolean alwaysSecure, boolean otherInput) {
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// BAD: secure flag not set
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// BAD: secure flag set to false
cookie.setSecure(false);
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// BAD: secure flag set to something not clearly true or request.isSecure()
cookie.setSecure(otherInput);
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// BAD: secure flag sometimes set to something clearly true or request.isSecure()
boolean secureVal;
if(alwaysSecure)
secureVal = true;
else
secureVal = otherInput;
cookie.setSecure(secureVal);
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// GOOD: set secure flag
cookie.setSecure(true);
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// GOOD: set secure flag
cookie.setSecure(true);
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// GOOD: set secure flag
cookie.setSecure(constTrue);
response.addCookie(cookie);
}
{
Cookie cookie = new Cookie("secret" ,"fakesecret");
// GOOD: set secure flag if contacted over HTTPS
cookie.setSecure(alwaysSecure ? true : request.isSecure());
response.addCookie(cookie);
}
}
}