Skip to content

Conversation

@ggolawski
Copy link
Contributor

@ggolawski ggolawski commented Jan 3, 2020

Spring has built-in CSRF protection. However, it's possible to disable it, which most likely makes the application vulnerable to Cross-Site Request Forgery (CWE-352).

Spring CSRF protection can be disabled via Java configuration (more details here):

@Configuration
@EnableWebSecurity
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {
  @Override
  protected void configure(HttpSecurity http) {
    http
      .csrf(csrf ->
        csrf.disable()
      );
  }
}

This PR adds a CodeQL check which finds all invocations of CsrfConfigurer.disable() method.

@ggolawski ggolawski requested review from a team and felicitymay as code owners January 3, 2020 21:02
@ghost
Copy link

ghost commented Jan 3, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including a help file with your query. Generally it looks good but I've made a few suggestions with the aim of simplifying the English (shorter sentence) and clarifying a few things.

Fix the help according to review comments.
@ggolawski
Copy link
Contributor Author

Thanks for reviewing this PR. I applied your suggestions.

Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for those changes. On re-reading the qhelp file, I realized that one of my suggestions had removed the context from the start of the Recommendation section. 😞

I've made a suggestion to fix this. Apart from this issue, the text looks ready to merge.

@aschackmull
Copy link
Contributor

Could you also run autoformat on the QL code? (That's available in the right-click menu in VSCode)

Fix help and correct formatting.
@ggolawski
Copy link
Contributor Author

I corrected the help and formatted the QL code.

Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for correcting the help 😄

@aschackmull aschackmull merged commit 816a8d1 into github:master Jan 27, 2020
@ggolawski ggolawski deleted the spring_disable_csrf branch January 27, 2020 19:37
@yo-h yo-h added the Java label May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants