Skip to content

Multipartfile request with no authentication is still consumed even after an AccessDeniedException is thrown #7060

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

Closed
a-sayyed opened this issue Jul 1, 2019 · 4 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Milestone

Comments

@a-sayyed
Copy link
Contributor

a-sayyed commented Jul 1, 2019

Summary

As explained by @wilkinsona in this related issue: spring-projects/spring-boot#17345, a multipartfile request with no authentication to a secure endpoint, results in an AccessDeniedException (when the HiddenHttpMethodFilter is disabled). This is handled in ExceptionTranslationFilter.handleSpringSecurityException(HttpServletRequest, HttpServletResponse, FilterChain, RuntimeException) which results in the creation of a DefaultSavedRequest. This calls javax.servlet.ServletRequest.getParameterMap() which causes the multipart request to be consumed and parsed.

Actual Behavior

Multipart file request with no Authentication to a secure endpoint results in the request being parsed and consumed anyway, then the client gets a 401 Unauthorized

Expected Behavior

Multipart file request with no Authentication to a secure endpoint should result in the request not being parsed or consumed, and the client gets a 401 Unauthorized as soon as an AccessDeniedException is thrown.

Configuration

Please see the attached sample. You will need to add the property spring.mvc.hiddenmethod.filter.enabled=false to the application.properties file

Version

2.1.6.RELEASE

Sample

https://github.com/a-sayyed/spring-jetty-secure-multipartfile-upload-bug
2019-06-28_09h29_33

@rwinch
Copy link
Member

rwinch commented Jul 3, 2019

Thanks for the report @a-sayyed!

I'm not sure this can be avoided all together. This is a common issue for multipart requests. For example, CSRF needs to process the parameters to determine if the request is valid.

So I can better understand your issue, can you please explain what specific problem this is causing you? I understand a file can be uploaded, but this doesn't seem any more of a problem than a user can make requests to the server (the difference is the file is written to disk vs just being in memory).

@rwinch rwinch added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 3, 2019
@rwinch rwinch self-assigned this Jul 3, 2019
@a-sayyed
Copy link
Contributor Author

a-sayyed commented Jul 9, 2019

Thank you for the explanation @rwinch!
My problem is exactly what you mentioned, I understand that this is similar to normal "bad requests", however this can quite easily result in a DDoS attack. Is it not possible to move the Multipartfilter behind the security filter chain? would that solve this issue?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 9, 2019
@rwinch
Copy link
Member

rwinch commented Jul 16, 2019

It isn't possible when security needs to read anything from the body (i.e. CSRF parameter, form body, etc).

While I understand this reduces the risk, it seems that if you are in trouble for a DoS attack, you still have this problem if the user is authorized to make the request (many attacks are user's with permissions).

If we modify the request to ignore saving the parameters, then a user that has the session time out while filling out a multipart form is likely to run into an issue where a page requires certain parameters and the saved request no longer has them which would result in surprising errors.

Instead, I'd suggest that we modify the configured HttpSessionRequestCache.requestMatcher to ignore multipart requests. Would you be interested in submitting a PR for it?

In the meantime, you can explicitly create and configure a HttpSessionRequestCache that ignores multi part requests.

@rwinch rwinch added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement status: ideal-for-contribution An issue that we actively are looking for someone to help us with and removed status: feedback-provided Feedback has been provided labels Jul 16, 2019
@a-sayyed
Copy link
Contributor Author

Hi @rwinch sounds good!, I will let you know once I am done 👍

@rwinch rwinch removed their assignment Jul 29, 2019
@rwinch rwinch closed this as completed in 71444ff Aug 15, 2019
@rwinch rwinch self-assigned this Aug 15, 2019
@rwinch rwinch removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Aug 15, 2019
@rwinch rwinch added this to the 5.2.0.RC1 milestone Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants