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

[Security] Fix potential XSS on /view #6034

Merged
merged 1 commit into from
Dec 13, 2024
Merged

[Security] Fix potential XSS on /view #6034

merged 1 commit into from
Dec 13, 2024

Conversation

huchenlei
Copy link
Collaborator

@huchenlei huchenlei commented Dec 13, 2024

This PR prevents html/htm/js/css files being served as web page for code execution.

Reproduction steps

  • Put a index.html file with random content in ComfyUI/output folder
  • Run ComfyUI
  • Type http://localhost:8188/api/view?filename=index.html&type=output&subfolder= in browser nav bar
  • Observe that the page is getting loaded. With the patch, you should observe that the html file is being downloaded instead.

Note

During testing, make sure that you disable browser cache between different testing procedures.

@huchenlei huchenlei marked this pull request as ready for review December 13, 2024 02:46
@Kosinkadink
Copy link
Collaborator

Works as expected on my machine - downloads the .html file instead of running it.

Not sure how much effort it would be, but is the api/upload/image endpoint expected to allow non-image types to be uploaded as well, or can it be modified to not allow .html uploads? The xss here at its core is the stored/persistent kind, so getting rid of the .html being uploaded in the first place would prevent the vulnerability from being exploited by another endpoint in the future.

@Kosinkadink
Copy link
Collaborator

For archiving purposes, here is the vulnerability this PR aims to resolve: https://nvd.nist.gov/vuln/detail/CVE-2024-10099

@comfyanonymous comfyanonymous merged commit 59d58b1 into master Dec 13, 2024
6 checks passed
@comfyanonymous comfyanonymous deleted the fix_xss branch December 13, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants