Skip to content

Commit

Permalink
[#144] Do not generate page tokens for pages that are not protected
Browse files Browse the repository at this point in the history
* Also do not try to inject into forms with GET HTTP method (inject-get-forms) if GET is configured to be an un-protected method by configuration (org.owasp.csrfguard.UnprotectedMethods)
  • Loading branch information
forgedhallpass committed Sep 10, 2020
1 parent 5a6e85d commit 84fb53d
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 20 deletions.
2 changes: 1 addition & 1 deletion csrfguard/src/main/java/org/owasp/csrfguard/CsrfGuard.java
Expand Up @@ -278,7 +278,7 @@ && isTokenPerPagePrecreate()
}

/**
* Method to be called by a logical session implementation when a session is destoryed. <br>
* Method to be called by a logical session implementation when a session is destroyed. <br>
*
* Example: {@link javax.servlet.http.HttpSessionListener#sessionDestroyed(javax.servlet.http.HttpSessionEvent)}
*
Expand Down
Expand Up @@ -89,8 +89,7 @@ private void handleSession(final HttpServletRequest httpServletRequest, final In
logInvalidRequest(httpServletRequest, csrfGuard);
}

// FIXME who and when is going to send this back to the UI?
csrfGuard.getTokenService().generateTokensIfAbsent(logicalSessionKey, httpServletRequest.getRequestURI());
csrfGuard.getTokenService().generateTokensIfAbsent(logicalSessionKey, httpServletRequest.getMethod(), httpServletRequest.getRequestURI());
}

private void handleNoSession(final HttpServletRequest httpServletRequest, final HttpServletResponse httpServletResponse, final InterceptRedirectResponse interceptRedirectResponse, final FilterChain filterChain, final CsrfGuard csrfGuard) throws IOException, ServletException {
Expand Down Expand Up @@ -122,6 +121,6 @@ private void logInvalidRequest(final HttpServletRequest httpRequest, final CsrfG
final String requestURI = httpRequest.getRequestURI();
final String remoteAddress = httpRequest.getRemoteAddr();

csrfGuard.getLogger().log(LogLevel.Warning, String.format("Invalid request: \r\nURI:\r\n%s\r\nRemote Address:%s", requestURI, remoteAddress));
csrfGuard.getLogger().log(LogLevel.Warning, String.format("Invalid request: \r\nURI: \r\n%s\r\n Remote Address: %s", requestURI, remoteAddress));
}
}
Expand Up @@ -48,6 +48,7 @@
import java.util.*;
import java.util.function.IntPredicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

/**
* {@link ConfigurationProvider} based on a {@link java.util.Properties} object.
Expand Down Expand Up @@ -336,7 +337,8 @@ public boolean isEnabled() {
@Override
public boolean isJavascriptInjectGetForms() {
this.javascriptInitParamsIfNeeded();
return this.javascriptInjectGetForms;

return this.javascriptInjectGetForms && getUnprotectedMethods().stream().noneMatch(unProtectedMethod -> unProtectedMethod.equalsIgnoreCase("GET"));
}

@Override
Expand Down Expand Up @@ -387,8 +389,8 @@ private Map<String, IAction> instantiateActions(final Properties properties) thr
}

private void initializeMethodProtection(final Properties properties) {
initializeMethodProtection(properties, ConfigParameters.PROTECTED_METHODS, this.protectedMethods);
initializeMethodProtection(properties, ConfigParameters.UNPROTECTED_METHODS, this.unprotectedMethods);
this.protectedMethods.addAll(initializeMethodProtection(properties, ConfigParameters.PROTECTED_METHODS));
this.unprotectedMethods.addAll(initializeMethodProtection(properties, ConfigParameters.UNPROTECTED_METHODS));

final HashSet<String> intersection = new HashSet<>(this.protectedMethods);
intersection.retainAll(this.unprotectedMethods);
Expand All @@ -398,13 +400,11 @@ private void initializeMethodProtection(final Properties properties) {
}
}

private static void initializeMethodProtection(final Properties properties, final String protectedMethods, final Set<String> protectedMethods2) {
final String protectedMethodList = PropertyUtils.getProperty(properties, protectedMethods);
if (StringUtils.isNotBlank(protectedMethodList)) {
for (final String method : protectedMethodList.split(",")) {
protectedMethods2.add(method.trim());
}
}
private static Set<String> initializeMethodProtection(final Properties properties, final String configParameterName) {
final String methodProtectionValue = PropertyUtils.getProperty(properties, configParameterName);
// TODO create HTTP method enum and (ignore-case) validate the provided values against it
return StringUtils.isNotBlank(methodProtectionValue) ? Arrays.stream(methodProtectionValue.split(",")).map(String::trim).collect(Collectors.toSet())
: Collections.emptySet();
}

private void initializePageProtection(final Properties properties) {
Expand Down
Expand Up @@ -101,6 +101,6 @@ private String computeTokenValue(final String locationUri) {

final LogicalSession logicalSession = this.csrfGuard.getLogicalSessionExtractor().extract(this.request);

return Objects.nonNull(logicalSession) ? tokenService.generateTokensIfAbsent(logicalSession.getKey(), locationUri) : null;
return Objects.nonNull(logicalSession) ? tokenService.generateTokensIfAbsent(logicalSession.getKey(), "GET", locationUri) : null;
}
}
Expand Up @@ -97,19 +97,25 @@ public Map<String, String> getPageTokens(final String logicalSessionKey) {
return new HashMap<>(tokenHolder.getPageTokens(logicalSessionKey));
}

/**
/**
* Generates master token and page token for the current resource if the token-per-page configuration is enabled
* <p>
*
* @param logicalSessionKey identifies the current logical session uniquely
* @param requestURI the URI of the desired HTTP resource
* @param httpMethod the current HTTP method used to request the resource
* @param requestURI the URI of the desired HTTP resource
* @return returns the generated page or master token
*/
public String generateTokensIfAbsent(final String logicalSessionKey, final String requestURI) {
public String generateTokensIfAbsent(final String logicalSessionKey, final String httpMethod, final String requestURI) {
final TokenHolder tokenHolder = this.csrfGuard.getTokenHolder();

return this.csrfGuard.isTokenPerPageEnabled() ? tokenHolder.createPageTokenIfAbsent(logicalSessionKey, CsrfGuardUtils.normalizeResourceURI(requestURI), TokenUtils::generateRandomToken)
: tokenHolder.createMasterTokenIfAbsent(logicalSessionKey, TokenUtils::generateRandomToken);
if (this.csrfGuard.isTokenPerPageEnabled()) {
if (this.csrfGuard.isProtectedPageAndMethod(requestURI, httpMethod)) {
return tokenHolder.createPageTokenIfAbsent(logicalSessionKey, CsrfGuardUtils.normalizeResourceURI(requestURI), TokenUtils::generateRandomToken);
}
}

return tokenHolder.createMasterTokenIfAbsent(logicalSessionKey, TokenUtils::generateRandomToken);
}

/**
Expand Down

0 comments on commit 84fb53d

Please sign in to comment.