Fix Multipart form processing: IllegalArgumentException: File path not allowed

[tjordahl]

Do not call SecurityUtils.getValidFilePath when processing a Multipart form as it will reject almost anything a user might put there. Just use the base filename

[sapessi]

Can you add null and null bytes check?

[acbron]

I changed the aws-serverless-java-container-spring to version 1.2, but it got NPE now. Is it still a bug ?

[sapessi]

Could you post the stack trace @acbron?

[acbron]

Could you post the stack trace @acbron?
@sapessi Here is the log

[main] ERROR com.amazonaws.serverless.proxy.internal.LambdaContainerHandler - Error while handling request
org.springframework.web.util.NestedServletException: Request processing failed; nested exception is org.springframework.web.multipart.MultipartException: Failed to parse multipart servlet request; nested exception is java.lang.NullPointerException
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:901)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:875)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
at com.amazonaws.serverless.proxy.spring.LambdaSpringApplicationInitializer.dispatch(LambdaSpringApplicationInitializer.java:114)
at com.amazonaws.serverless.proxy.spring.LambdaSpringApplicationInitializer.service(LambdaSpringApplicationInitializer.java:205)
at com.amazonaws.serverless.proxy.internal.servlet.FilterChainManager$ServletExecutionFilter.doFilter(FilterChainManager.java:351)
at com.amazonaws.serverless.proxy.internal.servlet.FilterChainHolder.doFilter(FilterChainHolder.java:84)
at com.amazonaws.serverless.proxy.internal.servlet.FilterChainHolder.doFilter(FilterChainHolder.java:84)
at com.amazonaws.serverless.proxy.internal.servlet.AwsLambdaServletContainerHandler.doFilter(AwsLambdaServletContainerHandler.java:207)
at com.amazonaws.serverless.proxy.spring.SpringLambdaContainerHandler.handleRequest(SpringLambdaContainerHandler.java:182)
at com.amazonaws.serverless.proxy.spring.SpringLambdaContainerHandler.handleRequest(SpringLambdaContainerHandler.java:42)
at com.amazonaws.serverless.proxy.internal.LambdaContainerHandler.proxy(LambdaContainerHandler.java:177)
at com.amazonaws.serverless.proxy.internal.LambdaContainerHandler.proxyStream(LambdaContainerHandler.java:209)
at com.worksap.company.hue.todo.handler.TodoStreamLambdaHandler.handleRequest(TodoStreamLambdaHandler.java:64)
at lambdainternal.EventHandlerLoader$2.call(EventHandlerLoader.java:888)
at lambdainternal.AWSLambda.startRuntime(AWSLambda.java:293)
at lambdainternal.AWSLambda.<clinit>(AWSLambda.java:64)
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:348)
at lambdainternal.LambdaRTEntry.main(LambdaRTEntry.java:104)
Caused by: org.springframework.web.multipart.MultipartException: Failed to parse multipart servlet request; nested exception is java.lang.NullPointerException
at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.handleParseFailure(StandardMultipartHttpServletRequest.java:123)
at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.parseRequest(StandardMultipartHttpServletRequest.java:114)
at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.<init>(StandardMultipartHttpServletRequest.java:87)
at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.<init>(StandardMultipartHttpServletRequest.java:71)
at org.springframework.web.multipart.support.MultipartResolutionDelegate.resolveMultipartArgument(MultipartResolutionDelegate.java:98)
at org.springframework.web.method.annotation.RequestParamMethodArgumentResolver.resolveName(RequestParamMethodArgumentResolver.java:164)
at org.springframework.web.method.annotation.AbstractNamedValueMethodArgumentResolver.resolveArgument(AbstractNamedValueMethodArgumentResolver.java:107)
at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:124)
at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:165)
at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138)
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:102)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:800)
at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1038)
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:942)
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:998)

The method for calling is like：

    @RequestMapping(value = "save", method = RequestMethod.POST)
    @ResponseBody
    public AttachmentData save(@RequestParam MultipartFile file) throws IOException {
        // save file to s3 
    }

However, I can guarantee there are no any NPE caused by my code, and it works fine in my local env. This happens only after i deployed it to API Gateway + lambda.

[sapessi]

Thanks @acbron. I'll try to replicate.

[sapessi]

Hey @acbron, I have been able to replicate this. It is indeed a bug. However, it is not related to this PR. The issue is caused by the fact that Spring tries to fetch the Content-Disposition header for each part and does not perform a null check on them. The bug in the library is that it wasn't setting the headers in each part. I have a fix for this and I'm building some tests for it. I'll reference this issue in the commit and we'll push out the changes with the next release.