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

Java: narrow query

Browse Source 
remove PUT and DELETE from StaplerCsrfUnprotectedMethod

remove OPTIONS and TRACE from SpringCsrfUnprotectedMethod
This commit is contained in:
Jami Cogswell
2025-01-29 21:30:53 -05:00
parent ead224c7b2
commit 530103e2d9
2 changed files with 19 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
java/ql/lib/semmle/code/java/security/CsrfUnprotectedRequestTypeQuery.qll
View File

@@ -25,7 +25,7 @@ private class SpringCsrfUnprotectedMethod extends CsrfUnprotectedMethod instance
or
this.hasAnnotation("org.springframework.web.bind.annotation", "RequestMapping") and
(
this.getMethodValue() = ["GET", "HEAD", "OPTIONS", "TRACE"]
this.getMethodValue() = ["GET", "HEAD"]
or
// If no request type is specified with `@RequestMapping`, then all request types
// are possible, so we treat this as unsafe; example: @RequestMapping(value = "test").
@@ -43,7 +43,10 @@ private class StaplerCsrfUnprotectedMethod extends CsrfUnprotectedMethod instanc
{
StaplerCsrfUnprotectedMethod() {
not this.hasAnnotation("org.kohsuke.stapler.interceptor", "RequirePOST") and
not this.hasAnnotation("org.kohsuke.stapler.verb", "POST")
// Jenkins only explicitly protects against CSRF for POST requests, but we
// also exclude PUT and DELETE since these request types are only exploitable
// if there is a CORS issue.
not this.hasAnnotation("org.kohsuke.stapler.verb", ["POST", "PUT", "DELETE"])
}
}

18
java/ql/test/query-tests/security/CWE-352/CsrfUnprotectedRequestTypeTest.java
View File

@@ -88,6 +88,17 @@ public class CsrfUnprotectedRequestTypeTest {
} catch (SQLException e) { }
}
// GOOD: uses OPTIONS or TRACE, which are unlikely to be exploitable via CSRF
@RequestMapping(value = "", method = { OPTIONS, TRACE })
public void good0() {
try {
String sql = "DELETE";
Connection conn = DriverManager.getConnection("url");
PreparedStatement ps = conn.prepareStatement(sql);
ps.executeUpdate(); // database update method call
} catch (SQLException e) { }
}
// GOOD: uses POST request when updating a database
@RequestMapping(value = "", method = RequestMethod.POST)
public void good1() {
@@ -430,11 +441,10 @@ public class CsrfUnprotectedRequestTypeTest {
return "post";
}
// BAD: Stapler web method annotated with `@PUT` and method name that implies a state-change
// We treat this case as bad for Stapler since the Jenkins docs only say that @POST/@RequirePOST
// provide default protection against CSRF.
// GOOD: Stapler web method annotated with `@PUT` and method name that implies a state-change
// We treat this case as good since PUT is only exploitable if there is a CORS issue.
@PUT
public String doPut(String user) { // $ hasCsrfUnprotectedRequestType
public String doPut(String user) {
return "put";
}