Skip to content

Commit f634f6e

Browse files
sapessisapessi
authored
Merge pull request #100 from awslabs/jersey-upgrade
Jersey switch to servlets and FindBugs pass
2 parents 0675628 + c40ac37 commit f634f6e

File tree

54 files changed

+1377
-721
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+1377
-721
lines changed

‎aws-serverless-java-container-core/pom.xml

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,78 @@
6565
</dependency>
6666
</dependencies>
6767

68+
<reporting>
69+
<plugins>
70+
<plugin>
71+
<groupId>org.codehaus.mojo</groupId>
72+
<artifactId>findbugs-maven-plugin</artifactId>
73+
<version>3.0.5</version>
74+
<configuration>
75+
<!--
76+
Enables analysis which takes more memory but finds more bugs.
77+
If you run out of memory, changes the value of the effort element
78+
to 'low'.
79+
-->
80+
<effort>Max</effort>
81+
<!-- Reports all bugs (other values are medium and max) -->
82+
<threshold>Low</threshold>
83+
<!-- Produces XML report -->
84+
<xmlOutput>true</xmlOutput>
85+
86+
<plugins>
87+
<plugin>
88+
<groupId>com.h3xstream.findsecbugs</groupId>
89+
<artifactId>findsecbugs-plugin</artifactId>
90+
<version>1.7.1</version>
91+
</plugin>
92+
</plugins>
93+
</configuration>
94+
</plugin>
95+
</plugins>
96+
</reporting>
97+
98+
<build>
99+
<plugins>
100+
<plugin>
101+
<groupId>org.codehaus.mojo</groupId>
102+
<artifactId>findbugs-maven-plugin</artifactId>
103+
<version>3.0.5</version>
104+
<configuration>
105+
<!--
106+
Enables analysis which takes more memory but finds more bugs.
107+
If you run out of memory, changes the value of the effort element
108+
to 'Low'.
109+
-->
110+
<effort>Max</effort>
111+
<!-- Reports all bugs (other values are medium and max) -->
112+
<threshold>Low</threshold>
113+
<!-- Produces XML report -->
114+
<xmlOutput>true</xmlOutput>
115+
<!-- Configures the directory in which the XML report is created -->
116+
<findbugsXmlOutputDirectory>${project.build.directory}/findbugs</findbugsXmlOutputDirectory>
117+
118+
<plugins>
119+
<plugin>
120+
<groupId>com.h3xstream.findsecbugs</groupId>
121+
<artifactId>findsecbugs-plugin</artifactId>
122+
<version>1.7.1</version>
123+
</plugin>
124+
</plugins>
125+
</configuration>
126+
<executions>
127+
<!--
128+
Ensures that FindBugs inspects source code when project is compiled.
129+
-->
130+
<execution>
131+
<id>analyze-compile</id>
132+
<phase>compile</phase>
133+
<goals>
134+
<goal>check</goal>
135+
</goals>
136+
</execution>
137+
</executions>
138+
</plugin>
139+
</plugins>
140+
</build>
141+
68142
</project>

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/AwsProxySecurityContextWriter.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@ public class AwsProxySecurityContextWriter implements SecurityContextWriter<AwsP
2828
// Variables - Private - Static
2929
//-------------------------------------------------------------
3030

31-
private static AwsProxySecurityContext currentContext;
31+
private AwsProxySecurityContext currentContext;
3232

3333

3434
//-------------------------------------------------------------
3535
// Implementation - SecurityContextWriter
3636
//-------------------------------------------------------------
3737

3838
public SecurityContext writeSecurityContext(AwsProxyRequest event, Context lambdaContext) {
39-
currentContext = new AwsProxySecurityContext(lambdaContext, event);
39+
currentContext = new AwsProxySecurityContext(lambdaContext, event);
4040

4141
return currentContext;
4242
}
@@ -46,7 +46,7 @@ public SecurityContext writeSecurityContext(AwsProxyRequest event, Context lambd
4646
// Methods - Getter/Setter
4747
//-------------------------------------------------------------
4848

49-
public static AwsProxySecurityContext getCurrentContext() {
49+
public AwsProxySecurityContext getCurrentContext() {
5050
return currentContext;
5151
}
5252
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/RequestReader.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ public abstract class RequestReader<RequestType, ContainerRequestType> {
5050
*/
5151
public static final String LAMBDA_CONTEXT_PROPERTY = "com.amazonaws.lambda.context";
5252

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

5459
//-------------------------------------------------------------
5560
// Methods - Abstract

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/ResponseWriter.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616
import com.amazonaws.serverless.exceptions.InvalidResponseObjectException;
1717
import com.amazonaws.services.lambda.runtime.Context;
1818

19-
import com.fasterxml.jackson.databind.ObjectMapper;
20-
21-
import java.io.IOException;
22-
import java.io.OutputStream;
19+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2320

2421

2522
/**
@@ -52,6 +49,7 @@ public abstract ResponseType writeResponse(ContainerResponseType containerRespon
5249
* @param input The byte[] to check against
5350
* @return true if the contend is valid UTF-8, false otherwise
5451
*/
52+
@SuppressFBWarnings("NS_NON_SHORT_CIRCUIT")
5553
protected boolean isValidUtf8(final byte[] input) {
5654
int i = 0;
5755
// Check for BOM

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/LambdaContainerHandler.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public abstract class LambdaContainerHandler<RequestType, ResponseType, Containe
6565
//-------------------------------------------------------------
6666

6767
private static ContainerConfig config = ContainerConfig.defaultConfig();
68-
private static ObjectMapper objectMapper;
68+
private static volatile ObjectMapper objectMapper;
6969

7070

7171

@@ -115,7 +115,6 @@ public static ObjectMapper getObjectMapper() {
115115
* @param basePath The base path to be stripped from the request
116116
*/
117117
public void stripBasePath(String basePath) {
118-
log.debug("Setting framework to strip base path: " + basePath);
119118
config.setStripBasePath(true);
120119
config.setServiceBasePath(basePath);
121120
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/SecurityUtils.java

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
package com.amazonaws.serverless.proxy.internal;
2+
3+
4+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
5+
import org.slf4j.Logger;
6+
import org.slf4j.LoggerFactory;
7+
8+
import java.io.File;
9+
import java.io.IOException;
10+
import java.util.List;
11+
import java.util.Locale;
12+
13+
14+
/**
15+
* This class contains utility methods to address FSB security issues found in the application, such as string sanitization
16+
* and file path validation.
17+
*/
18+
public final class SecurityUtils {
19+
private static Logger log = LoggerFactory.getLogger(SecurityUtils.class);
20+
21+
/**
22+
* Replaces CRLF characters in a string with empty string ("").
23+
* @param s The string to be cleaned
24+
* @return A copy of the original string without CRLF characters
25+
*/
26+
public static String crlf(String s) {
27+
return s.replaceAll("[\r\n]", "");
28+
}
29+
30+
31+
/**
32+
* Escapes all special characters in a java string
33+
* @param s The string to be cleaned
34+
* @return The escaped string
35+
*/
36+
public static String encode(String s) {
37+
if (s == null) {
38+
return null;
39+
}
40+
41+
int sz = s.length();
42+
43+
StringBuffer buffer = new StringBuffer();
44+
for (int i = 0; i < sz; i++) {
45+
char ch = s.charAt(i);
46+
47+
// handle unicode
48+
if (ch > 0xfff) {
49+
buffer.append("\\u" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
50+
} else if (ch > 0xff) {
51+
buffer.append("\\u0" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
52+
} else if (ch > 0x7f) {
53+
buffer.append("\\u00" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
54+
} else if (ch < 32) {
55+
switch (ch) {
56+
case '\b':
57+
buffer.append('\\');
58+
buffer.append('b');
59+
break;
60+
case '\n':
61+
buffer.append('\\');
62+
buffer.append('n');
63+
break;
64+
case '\t':
65+
buffer.append('\\');
66+
buffer.append('t');
67+
break;
68+
case '\f':
69+
buffer.append('\\');
70+
buffer.append('f');
71+
break;
72+
case '\r':
73+
buffer.append('\\');
74+
buffer.append('r');
75+
break;
76+
default:
77+
if (ch > 0xf) {
78+
buffer.append("\\u00" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
79+
} else {
80+
buffer.append("\\u000" + Integer.toHexString(ch).toUpperCase(Locale.ENGLISH));
81+
}
82+
break;
83+
}
84+
} else {
85+
switch (ch) {
86+
case '\'':
87+
88+
buffer.append('\'');
89+
break;
90+
case '"':
91+
buffer.append('\\');
92+
buffer.append('"');
93+
break;
94+
case '\\':
95+
buffer.append('\\');
96+
buffer.append('\\');
97+
break;
98+
case '/':
99+
buffer.append('/');
100+
break;
101+
default:
102+
buffer.append(ch);
103+
break;
104+
}
105+
}
106+
}
107+
108+
return buffer.toString();
109+
}
110+
111+
public static String getValidFilePath(String inputPath) {
112+
return getValidFilePath(inputPath, false);
113+
}
114+
115+
/**
116+
* Returns an absolute file path given an input path and validates that it is not trying
117+
* to write/read from a directory other than /tmp.
118+
* @param inputPath The input path
119+
* @return The absolute path to the file
120+
* @throws IllegalArgumentException If the given path is not valid or outside of /tmp
121+
*/
122+
@SuppressFBWarnings("PATH_TRAVERSAL_IN")
123+
public static String getValidFilePath(String inputPath, boolean isWrite) {
124+
if (inputPath == null || "".equals(inputPath.trim())) {
125+
return null;
126+
}
127+
128+
File f = new File(inputPath);
129+
try {
130+
String canonicalPath = f.getCanonicalPath();
131+
132+
if (isWrite && canonicalPath.startsWith("/var/task")) {
133+
throw new IllegalArgumentException("Trying to write to /var/task folder");
134+
}
135+
136+
boolean isAllowed = false;
137+
for (String allowedPath : LambdaContainerHandler.getContainerConfig().getValidFilePaths()) {
138+
if (canonicalPath.startsWith(allowedPath)) {
139+
isAllowed = true;
140+
break;
141+
}
142+
}
143+
if (!isAllowed) {
144+
throw new IllegalArgumentException("File path not allowed: " + encode(canonicalPath));
145+
}
146+
147+
return canonicalPath;
148+
} catch (IOException e) {
149+
log.error("Invalid file path: {}", encode(inputPath));
150+
throw new IllegalArgumentException("Invalid file path", e);
151+
}
152+
}
153+
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/jaxrs/AwsProxySecurityContext.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,19 @@ public class AwsProxySecurityContext
4343
// Variables - Private
4444
//-------------------------------------------------------------
4545

46-
protected Context lambdaContext;
47-
protected AwsProxyRequest event;
46+
private Context lambdaContext;
47+
private AwsProxyRequest event;
4848

4949

50+
public Context getLambdaContext() {
51+
return lambdaContext;
52+
}
53+
54+
55+
public AwsProxyRequest getEvent() {
56+
return event;
57+
}
58+
5059
//-------------------------------------------------------------
5160
// Constructors
5261
//-------------------------------------------------------------
@@ -119,7 +128,7 @@ public String getAuthenticationScheme() {
119128
* Custom object for request authorized with a Cognito User Pool authorizer. By casting the Principal
120129
* object to this you can extract the Claims object included in the token.
121130
*/
122-
public class CognitoUserPoolPrincipal implements Principal {
131+
public static class CognitoUserPoolPrincipal implements Principal {
123132

124133
private CognitoAuthorizerClaims claims;
125134

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package com.amazonaws.serverless.proxy.internal.servlet;
1414

1515
import com.amazonaws.serverless.proxy.RequestReader;
16+
import com.amazonaws.serverless.proxy.internal.SecurityUtils;
1617
import com.amazonaws.serverless.proxy.model.ApiGatewayRequestContext;
1718
import com.amazonaws.serverless.proxy.model.ContainerConfig;
1819
import com.amazonaws.services.lambda.runtime.Context;
@@ -31,6 +32,7 @@
3132
import java.net.URLDecoder;
3233
import java.net.URLEncoder;
3334
import java.nio.charset.StandardCharsets;
35+
import java.security.Security;
3436
import java.util.AbstractMap;
3537
import java.util.ArrayList;
3638
import java.util.Collections;
@@ -335,7 +337,7 @@ protected String decodeRequestPath(String requestPath, ContainerConfig config) {
335337
try {
336338
return URLDecoder.decode(requestPath, config.getUriEncoding());
337339
} catch (UnsupportedEncodingException ex) {
338-
log.error("Could not URL decode the request path, configured encoding not supported: " + config.getUriEncoding(), ex);
340+
log.error("Could not URL decode the request path, configured encoding not supported: {}", SecurityUtils.encode(config.getUriEncoding()));
339341
// we do not fail at this.
340342
return requestPath;
341343
}

0 commit comments

Comments
 (0)