-
Notifications
You must be signed in to change notification settings - Fork 78
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
Security issue with next/image #19
Comments
Nice find, it makes a ton of sense! I think for now the best way to fix this is with documentation. I'd absolutely merge a PR that looks something like...
I have some ideas on how to fix this on the server side upload signer, but I think docs fix is a good first step. Btw, do you have a reproduction of the XSS? Something that does |
Ok I can confirm this worked... I was able to XSS an html upload. Thanks for the report! I've updated the README to mention only whitelisting image files with next/image. If we ever run into each other at a conference introduce yourself! I owe you a beer! :) |
Awesome, thank you! I was intending to set up a minimal repo to reproduce for you but got bogged down with other work. Thanks for following up without me 🙂 |
Hi @ryanto we just had a penetration test on our app which uses next-s3-upload on the frontend. It was possible upload a payload with an incubated JavaScript payload. While the document or image technically passed initial file validation, the platform did not check the contents of the file. Moreover, this meant that documents containing malware payloads would also be uploaded. The finding affected all file upload functions across the web application. Following the client browser obtaining a valid Security Token and upload URL, the below requests would be made to an S3 bucket: ==REQUEST = File Upload== References Do you have any suggestions or recommendations for preventing XSS attacks via document uploads with your package, or is this something we should be handling during the backend file sanitisation? |
Hi @leafyshark Sorry you're running into this. With browser based uploads its going to be up to you how you want to validate the contents of the uploaded files. One thing you can do is use a lambda to check files as they come into your bucket. From a security point of view, it's best to treat all uploaded files as potentially dangerous. At the end of the day you never know what your users are going to upload. With this in mind you could let users upload files to S3, use a lambda to check them, and then only after you've approved the upload make the files on S3 accessible to the rest of your users. Also, if you need to read the files before uploading them to S3 I think that client based uploading might not be the best approach. You'll be better off with uploading files to a server, scanning them, and then putting them on S3 after your server approves. PS: Let me know if you have any questions and I'll try my best to answer them! |
Hi thanks for the library, it has definitely saved me some hours of work! I'm not sure if this is within scope of the project, but should probably be mentioned in the docs. With the current permissions described set for the AWS role, the user can upload any sort of file.
In the common use case of displaying an uploaded image right away using the next/image component, Next will generate an optimized/cached version of the file that will be available at url like
<site domain>/_next/image?url=<uri encoded original image url>
. If the file uploaded is not an actual image file Next will return the file as it was uploaded. This leads to a potential XSS vulnerability because a user can upload an html file that will load at the URL described above and can run arbitrary javascript as though it were the site. Obviously checks can be done on the client to make sure that the file is the correct type, but if a bad actor wanted to, I believe (though not entirely sure) they could still use the headers from a valid request to make another authenticated call to upload a malicious file and manually generate the link to access it through the next site.My suggestion for this is simply restricting the permissions of the IAM role set up for the uploader to the allowed file types. Something like:
Let me know if that makes sense and thanks again!
The text was updated successfully, but these errors were encountered: