Java: add CSRF query

[jcogs33]

Description

Adds a query for CSRF vulnerabilities caused by using HTTP request types that are not default-protected by a given framework for apparent state-changing actions. Supports Spring and Stapler frameworks for now. State-changing actions are determined heuristically based on whether the request handling method updates a database or whether its name suggests that it might change application state. Due to the heuristic nature of determining state changes, this is a low precision query.

Consideration

• Let me know if you have any suggestions for improving the heuristics used for identifying state changes.
• See inline comments/questions regarding some code structure that should maybe be improved.
• Let me know if you have any suggestions for better ways to word the query name/id/description/alert message/qhelp. I went through a few iterations of different wording for these, and I'm not sure if what I ended up with is clear enough.
• Note: I am still looking into adding this query to autofix.

Pull Request checklist

All query authors

• A change note is added if necessary. See the documentation in this repository.
• All new queries have appropriate .qhelp. See the documentation in this repository.
• QL tests are added if necessary. See Testing custom queries in the GitHub documentation.
• New and changed queries have correct query metadata. See the documentation in this repository.

Internal query authors only

• Changes are validated at scale (internal access required).
• Adding a new query? Consider also adding the query to autofix.
• Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required).

[github-actions]

QHelp previews:

java/ql/src/Security/CWE/CWE-352/CsrfUnprotectedRequestType.qhelp

HTTP request type unprotected from CSRF

When you set up a web server to receive a request from a client without any mechanism for verifying that it was intentionally sent, then it is vulnerable to attack. An attacker can trick a client into making an unintended request to the web server that will be treated as an authentic request. This can be done via a URL, image load, XMLHttpRequest, etc. and can result in exposure of data or unintended code execution.

Recommendation

When handling requests, make sure any requests that change application state are protected from Cross Site Request Forgery (CSRF). Some application frameworks, such as Spring, provide default CSRF protection for HTTP request types that may change application state, such as POST. Other HTTP request types, such as GET, should not be used for actions that change the state of the application, since these request types are not default-protected from CSRF by the framework.

Example

The following example shows a Spring request handler using a GET request for a state-changing action. Since a GET request does not have default CSRF protection in Spring, this type of request should not be used when modifying application state. Instead use one of Spring's default-protected request types, such as POST.

// BAD - a GET request should not be used for a state-changing action like transfer
@RequestMapping(value="transfer", method=RequestMethod.GET)
public boolean transfer(HttpServletRequest request, HttpServletResponse response){
  return doTransfer(request, response);
}

// GOOD - use a POST request for a state-changing action
@RequestMapping(value="transfer", method=RequestMethod.POST)
public boolean transfer(HttpServletRequest request, HttpServletResponse response){
  return doTransfer(request, response);
}

References

• OWASP: Cross-Site Request Forgery (CSRF).
• Spring Security Reference: Cross Site Request Forgery (CSRF) .
• Common Weakness Enumeration: CWE-352.

[jcogs33]

I added pragma[only_bind_into] here to fix performance. Let me know if there is a better way to resolve this performance problem.

[jcogs33]

Let me know if there is a better way to handle this exclusion.