Skip to content

Commit

Permalink
SONARJAVA-4875 S4502: Improve support for various CSRF disabling calls (
Browse files Browse the repository at this point in the history
  • Loading branch information
johann-beleites-sonarsource authored Apr 2, 2024
1 parent 472b38f commit 9c0d6c7
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S1874",
"hasTruePositives": true,
"falseNegatives": 114,
"falseNegatives": 150,
"falsePositives": 0
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"ruleKey": "S4502",
"hasTruePositives": false,
"falseNegatives": 5,
"falseNegatives": 11,
"falsePositives": 0
}
4 changes: 2 additions & 2 deletions java-checks-test-sources/default/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,13 @@
<dependency>
<groupId>org.springframework.security</groupId>
<artifactId>spring-security-web</artifactId>
<version>5.2.10.RELEASE</version>
<version>5.8.11</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.springframework.security</groupId>
<artifactId>spring-security-config</artifactId>
<version>5.4.0</version>
<version>5.8.11</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
package checks.spring;

import javax.servlet.http.HttpServletRequest;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer;
import org.springframework.security.config.annotation.web.configurers.LogoutConfigurer;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.util.matcher.RequestMatcher;

@EnableWebSecurity
public class SpringSecurityDisableCSRFCheck extends WebSecurityConfigurerAdapter {
Expand All @@ -29,4 +35,26 @@ protected void configure(HttpSecurity http) throws Exception {
CsrfConfigurer<HttpSecurity> csrf2 = http.csrf();
csrf2.ignoringAntMatchers("/ignored/path"); // Noncompliant [[sc=11;ec=30]] {{Make sure disabling Spring Security's CSRF protection is safe here.}}
}

@Configuration
public static class CSRFConfig {

@Bean
public SecurityFilterChain filterChainCSRF(HttpSecurity http) throws Exception {
RequestMatcher requestMatcherWhitelist = (HttpServletRequest request) -> request.getRequestURI().contains("whitelist");
RequestMatcher requestMatcherBlacklist = (HttpServletRequest request) -> request.getRequestURI().contains("blacklist");

http.csrf(csrf -> csrf.requireCsrfProtectionMatcher(requestMatcherWhitelist)); // Noncompliant
http.csrf(csrf -> csrf.ignoringRequestMatchers(requestMatcherBlacklist)); // Noncompliant
http.csrf(csrf -> csrf.ignoringRequestMatchers("/S4502CSRFSpecial")); // Noncompliant
http.csrf(csrf -> csrf.ignoringAntMatchers("/ignored/path")); // Noncompliant
http.csrf(AbstractHttpConfigurer::disable); // Noncompliant
http.csrf(csrf -> csrf.disable()); // Noncompliant

http.cors(cors -> cors.disable()); // Compliant - not CSRF
http.cors(AbstractHttpConfigurer::disable); // Compliant - not CSRF

return http.build();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,25 @@
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodReferenceTree;
import org.sonar.plugins.java.api.tree.Tree;

@Rule(key = "S4502")
public class SpringSecurityDisableCSRFCheck extends AbstractMethodDetection {

private static final String CSRF_CONFIGURER_CLASS = "org.springframework.security.config.annotation.web.configurers.CsrfConfigurer";
private static final String MESSAGE = "Make sure disabling Spring Security's CSRF protection is safe here.";
private static final MethodMatchers DISABLE_MATCHER = MethodMatchers.create()
.ofSubTypes("org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer")
.names("disable")
.addWithoutParametersMatcher()
.build();
private static final MethodMatchers IGNORE_ANT_MATCHER = MethodMatchers.create()
.ofSubTypes("org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer")
.names("ignoringAntMatchers")
.addParametersMatcher("java.lang.String[]")
.build();

private static final MethodMatchers DISALLOWED_METHODS = MethodMatchers.create()
.ofSubTypes("org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer")
.names("disable", "ignoringAntMatchers", "requireCsrfProtectionMatcher", "ignoringRequestMatchers")
.withAnyParameters()
.build();


@Override
protected MethodMatchers getMethodInvocationMatchers() {
return MethodMatchers.or(
DISABLE_MATCHER,
IGNORE_ANT_MATCHER
);
return DISALLOWED_METHODS;
}

@Override
Expand All @@ -61,4 +55,11 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) {
}
}

@Override
protected void onMethodReferenceFound(MethodReferenceTree methodReferenceTree) {
var typeArgs = methodReferenceTree.symbolType().typeArguments();
if (typeArgs.size() == 1 && typeArgs.get(0).is(CSRF_CONFIGURER_CLASS)) {
reportIssue(methodReferenceTree.method(), MESSAGE);
}
}
}

0 comments on commit 9c0d6c7

Please sign in to comment.