Skip to content

Jersey switch to servlets and FindBugs pass #100

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

Merged
merged 5 commits into from
Jan 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions aws-serverless-java-container-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,78 @@
</dependency>
</dependencies>

<reporting>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>3.0.5</version>
<configuration>
<!--
Enables analysis which takes more memory but finds more bugs.
If you run out of memory, changes the value of the effort element
to 'low'.
-->
<effort>Max</effort>
<!-- Reports all bugs (other values are medium and max) -->
<threshold>Low</threshold>
<!-- Produces XML report -->
<xmlOutput>true</xmlOutput>

<plugins>
<plugin>
<groupId>com.h3xstream.findsecbugs</groupId>
<artifactId>findsecbugs-plugin</artifactId>
<version>1.7.1</version>
</plugin>
</plugins>
</configuration>
</plugin>
</plugins>
</reporting>

<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>3.0.5</version>
<configuration>
<!--
Enables analysis which takes more memory but finds more bugs.
If you run out of memory, changes the value of the effort element
to 'Low'.
-->
<effort>Max</effort>
<!-- Reports all bugs (other values are medium and max) -->
<threshold>Low</threshold>
<!-- Produces XML report -->
<xmlOutput>true</xmlOutput>
<!-- Configures the directory in which the XML report is created -->
<findbugsXmlOutputDirectory>${project.build.directory}/findbugs</findbugsXmlOutputDirectory>

<plugins>
<plugin>
<groupId>com.h3xstream.findsecbugs</groupId>
<artifactId>findsecbugs-plugin</artifactId>
<version>1.7.1</version>
</plugin>
</plugins>
</configuration>
<executions>
<!--
Ensures that FindBugs inspects source code when project is compiled.
-->
<execution>
<id>analyze-compile</id>
<phase>compile</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ public class AwsProxySecurityContextWriter implements SecurityContextWriter<AwsP
// Variables - Private - Static
//-------------------------------------------------------------

private static AwsProxySecurityContext currentContext;
private AwsProxySecurityContext currentContext;


//-------------------------------------------------------------
// Implementation - SecurityContextWriter
//-------------------------------------------------------------

public SecurityContext writeSecurityContext(AwsProxyRequest event, Context lambdaContext) {
currentContext = new AwsProxySecurityContext(lambdaContext, event);
currentContext = new AwsProxySecurityContext(lambdaContext, event);

return currentContext;
}
Expand All @@ -46,7 +46,7 @@ public SecurityContext writeSecurityContext(AwsProxyRequest event, Context lambd
// Methods - Getter/Setter
//-------------------------------------------------------------

public static AwsProxySecurityContext getCurrentContext() {
public AwsProxySecurityContext getCurrentContext() {
return currentContext;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ public abstract class RequestReader<RequestType, ContainerRequestType> {
*/
public static final String LAMBDA_CONTEXT_PROPERTY = "com.amazonaws.lambda.context";

/**
* The key for the <strong>JAX RS security context</strong> properties stored in the request attributes
*/
public static final String JAX_SECURITY_CONTEXT_PROPERTY = "com.amazonaws.serverless.jaxrs.securityContext";


//-------------------------------------------------------------
// Methods - Abstract
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@
import com.amazonaws.serverless.exceptions.InvalidResponseObjectException;
import com.amazonaws.services.lambda.runtime.Context;

import com.fasterxml.jackson.databind.ObjectMapper;

import java.io.IOException;
import java.io.OutputStream;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;


/**
Expand Down Expand Up @@ -52,6 +49,7 @@ public abstract ResponseType writeResponse(ContainerResponseType containerRespon
* @param input The byte[] to check against
* @return true if the contend is valid UTF-8, false otherwise
*/
@SuppressFBWarnings("NS_NON_SHORT_CIRCUIT")
protected boolean isValidUtf8(final byte[] input) {
int i = 0;
// Check for BOM
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public abstract class LambdaContainerHandler<RequestType, ResponseType, Containe
//-------------------------------------------------------------

private static ContainerConfig config = ContainerConfig.defaultConfig();
private static ObjectMapper objectMapper;
private static volatile ObjectMapper objectMapper;



Expand Down Expand Up @@ -115,7 +115,6 @@ public static ObjectMapper getObjectMapper() {
* @param basePath The base path to be stripped from the request
*/
public void stripBasePath(String basePath) {
log.debug("Setting framework to strip base path: " + basePath);
config.setStripBasePath(true);
config.setServiceBasePath(basePath);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package com.amazonaws.serverless.proxy.internal;


import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.Locale;


/**
* This class contains utility methods to address FSB security issues found in the application, such as string sanitization
* and file path validation.
*/
public final class SecurityUtils {
private static Logger log = LoggerFactory.getLogger(SecurityUtils.class);

/**
* Replaces CRLF characters in a string with empty string ("").
* @param s The string to be cleaned
* @return A copy of the original string without CRLF characters
*/
public static String crlf(String s) {
return s.replaceAll("[\r\n]", "");
}


/**
* Escapes all special characters in a java string
* @param s The string to be cleaned
* @return The escaped string
*/
public static String encode(String s) {
if (s == null) {
return null;
}

int sz = s.length();

StringBuffer buffer = new StringBuffer();
for (int i = 0; i < sz; i++) {
char ch = s.charAt(i);

// handle unicode
if (ch > 0xfff) {
buffer.append("\\u" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
} else if (ch > 0xff) {
buffer.append("\\u0" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
} else if (ch > 0x7f) {
buffer.append("\\u00" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
} else if (ch < 32) {
switch (ch) {
case '\b':
buffer.append('\\');
buffer.append('b');
break;
case '\n':
buffer.append('\\');
buffer.append('n');
break;
case '\t':
buffer.append('\\');
buffer.append('t');
break;
case '\f':
buffer.append('\\');
buffer.append('f');
break;
case '\r':
buffer.append('\\');
buffer.append('r');
break;
default:
if (ch > 0xf) {
buffer.append("\\u00" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
} else {
buffer.append("\\u000" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
}
break;
}
} else {
switch (ch) {
case '\'':

buffer.append('\'');
break;
case '"':
buffer.append('\\');
buffer.append('"');
break;
case '\\':
buffer.append('\\');
buffer.append('\\');
break;
case '/':
buffer.append('/');
break;
default:
buffer.append(ch);
break;
}
}
}

return buffer.toString();
}

public static String getValidFilePath(String inputPath) {
return getValidFilePath(inputPath, false);
}

/**
* Returns an absolute file path given an input path and validates that it is not trying
* to write/read from a directory other than /tmp.
* @param inputPath The input path
* @return The absolute path to the file
* @throws IllegalArgumentException If the given path is not valid or outside of /tmp
*/
@SuppressFBWarnings("PATH_TRAVERSAL_IN")
public static String getValidFilePath(String inputPath, boolean isWrite) {
if (inputPath == null || "".equals(inputPath.trim())) {
return null;
}

File f = new File(inputPath);
try {
String canonicalPath = f.getCanonicalPath();

if (isWrite && canonicalPath.startsWith("/var/task")) {
throw new IllegalArgumentException("Trying to write to /var/task folder");
}

boolean isAllowed = false;
for (String allowedPath : LambdaContainerHandler.getContainerConfig().getValidFilePaths()) {
if (canonicalPath.startsWith(allowedPath)) {
isAllowed = true;
break;
}
}
if (!isAllowed) {
throw new IllegalArgumentException("File path not allowed: " + encode(canonicalPath));
}

return canonicalPath;
} catch (IOException e) {
log.error("Invalid file path: {}", encode(inputPath));
throw new IllegalArgumentException("Invalid file path", e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,19 @@ public class AwsProxySecurityContext
// Variables - Private
//-------------------------------------------------------------

protected Context lambdaContext;
protected AwsProxyRequest event;
private Context lambdaContext;
private AwsProxyRequest event;


public Context getLambdaContext() {
return lambdaContext;
}


public AwsProxyRequest getEvent() {
return event;
}

//-------------------------------------------------------------
// Constructors
//-------------------------------------------------------------
Expand Down Expand Up @@ -119,7 +128,7 @@ public String getAuthenticationScheme() {
* Custom object for request authorized with a Cognito User Pool authorizer. By casting the Principal
* object to this you can extract the Claims object included in the token.
*/
public class CognitoUserPoolPrincipal implements Principal {
public static class CognitoUserPoolPrincipal implements Principal {

private CognitoAuthorizerClaims claims;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package com.amazonaws.serverless.proxy.internal.servlet;

import com.amazonaws.serverless.proxy.RequestReader;
import com.amazonaws.serverless.proxy.internal.SecurityUtils;
import com.amazonaws.serverless.proxy.model.ApiGatewayRequestContext;
import com.amazonaws.serverless.proxy.model.ContainerConfig;
import com.amazonaws.services.lambda.runtime.Context;
Expand All @@ -31,6 +32,7 @@
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.security.Security;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -335,7 +337,7 @@ protected String decodeRequestPath(String requestPath, ContainerConfig config) {
try {
return URLDecoder.decode(requestPath, config.getUriEncoding());
} catch (UnsupportedEncodingException ex) {
log.error("Could not URL decode the request path, configured encoding not supported: " + config.getUriEncoding(), ex);
log.error("Could not URL decode the request path, configured encoding not supported: {}", SecurityUtils.encode(config.getUriEncoding()));
// we do not fail at this.
return requestPath;
}
Expand Down
Loading