Skip to content
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

Spring CSRF: Protection Disabled & Unrestricted RequestMapping #261

Merged
merged 2 commits into from Jan 9, 2017
Merged

Spring CSRF: Protection Disabled & Unrestricted RequestMapping #261

merged 2 commits into from Jan 9, 2017

Conversation

ptamarit
Copy link
Contributor

@ptamarit ptamarit commented Jan 8, 2017

This pull request includes 2 related detectors, each in a separated commit.

SpringCsrfProtectionDisabledDetector

Recent versions of Spring Security protect by default Spring endpoints against CSRF attacks.
Since it does require some initial work from the developers on the client-side to make form submissions and/or AJAX requests work, it can be tempting to disable the CSRF protection temporarily in an initial development phase, with the risk of forgetting to re-enable it later on.

This detector detects the deactivation of Spring Security's CSRF protection via Spring JavaConfig.
It does not however detects the deactivation of CSRF protection via Spring XML config.

SpringCsrfUnrestrictedRequestMappingDetector

Although it can be customized, out of the box Spring Security doesn't protect endpoints with the HTTP request methods GET, HEAD, TRACE, and OPTIONS against CSRF attacks.

It's very easy to use @RequestMapping and to forget to restrict it to only the needed HTTP request methods. If this happens for a state-changing method, then this method will be vulnerable to CSRF attacks as the GET method will be mapped.

This detector detects unrestricted @RequestMapping, as well as unlikely suspicious cases where a user would restrict a given mapping to a mix of unprotected HTTP request methods (e.g. GET) and protected HTTP request methods (e.g. POST).

General Remarks

  • I was not really sure of where to put the new rules in findbugs.xml and messages.xml (as I didn't find any consistent logical ordering).
  • I chose to use high priorities for both detectors.

@h3xstream h3xstream merged commit ad14178 into find-sec-bugs:master Jan 9, 2017
@h3xstream
Copy link
Member

Looks good ! 👍

@h3xstream h3xstream self-assigned this Jan 9, 2017
@h3xstream h3xstream added this to the version-1.6.0 milestone Jan 9, 2017
@ptamarit ptamarit deleted the spring-csrf branch January 9, 2017 20:05
KengoTODA added a commit to KengoTODA/sonar-findbugs that referenced this pull request Jun 5, 2017
@h3xstream h3xstream added the enhancement New feature or improvement to existing detector. label Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing detector.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants