Skip to content
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

Vaadin 24 Upload Component setAcceptedFileTypes(...) only validating on client side #20409

Open
janarbis opened this issue Nov 5, 2024 · 6 comments

Comments

@janarbis
Copy link

janarbis commented Nov 5, 2024

Description of the bug

The file extension is only validated on client side when uploading a file via the vaadin upload component.

Expected behavior

Validation on client and server side. At least for the first POST request to the server.

Minimal reproducible example

  1. Create Upload component and setAcceptedFileTypes to pdf.
  2. Start interception in a proxy tool such as Burp Suite
  3. Select any PDF file and click on Upload
  4. Intercept the POST request made to server upload endpoint
  5. Replace the file content, file name and file type for example add an HTML code in the request for the POST request body and send the request
  6. You will notice that the HTML file will get upload in the server

Versions

  • Vaadin / Flow version: 24.4.13
  • Java version: 17
@Legioth
Copy link
Member

Legioth commented Nov 5, 2024

The elephant in the room is anyways that the client can send a request with the expected file name extension and MIME type but a file content that doesn't match that expectation. And there's nothing reasonable that Vaadin can do to inspect the file contents to check this.

From that point of view, I'm wonder if there's any practical impact from spoofing only the file name extension?

@janarbis
Copy link
Author

janarbis commented Nov 5, 2024

It is not only the extension. You can replace the entire file.
This was found and marked during a pen test of our application.

@Legioth
Copy link
Member

Legioth commented Nov 5, 2024

That's the other side of the elephant in the room: there's an infinite number of possible content types and it's not reasonable to expect that Vaadin will know how to verify the content for all of them. And even if Vaadin would do that e.g. for some common ones, there's still the possibility that e.g. a technically valid pdf file does still contain some malicious content that causes problems with some pdf viewers but not with others. Any content validation from our side would thus lead to an even falser sense of security than what there is today when there's not validation of the content.

I suspect that the only reasonable way is that it's up to the application to validate the content based on the application's own requirements.

@janarbis
Copy link
Author

janarbis commented Nov 5, 2024

Vaadin should not validate the content of the file. This is obviously not a good idea. Im speaking about the validation of the file extension on both sides if a white list is set. Server and client side. Otherwise it is possible to upload files that not match the white list if you intercept the client POST request as described in my first comment. But i can implement it by my self, no problem.

@Legioth
Copy link
Member

Legioth commented Nov 5, 2024

We tried doing that but it lead to some unexpected issues with the way the accepted file type can either be a file extension or a MIME type: #18185

We would have to deprecate the current API and instead introduce separate APIs for extensions and MIME types to solve that issue.

@knoobie
Copy link
Contributor

knoobie commented Nov 5, 2024

IMHO. I share both arguments, I would expect that the Upload does simple file extension checking itself and it is documented that it's the sole responsibility of the developer to implement proper MIME type and content validation on the server side - with e.g. something based on Apache Tika.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants