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

feat: add filename validation for windows #50

Closed
dominikg opened this issue Aug 18, 2022 · 6 comments
Closed

feat: add filename validation for windows #50

dominikg opened this issue Aug 18, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@dominikg
Copy link
Contributor

The vue-inspector by @webfansplz contains code that prevents some filenames from being opened with a shell command

https://github.com/webfansplz/vite-plugin-vue-inspector/blob/main/src/launch-editor.ts#L336-L357

This is to prevent possible attacks where a crafted filename could be used to execute malicious commands.

Currently launch-editor does not have a check like this. It's hard to exploit because there is an fs.existsSync check but for defense in depth it would be better to not let these kinds of filenames through

The regex here https://github.com/webfansplz/vite-plugin-vue-inspector/blob/main/src/launch-editor.ts#L108-L112 looks longish, maybe a smaller list of forbidden chars does the trick too

@haoqunjiang
Copy link
Collaborator

These are basically backports of facebook/create-react-app#4866 and facebook/create-react-app#5431
As Unicode character class escape has been supported since Node.js 101, I think we can use the uncompiled version here.

The only remaining problem is that this RegExp can't be parsed with our current ESLint setup.
I'll try to fix this issue when I have the time to move away from eslint-plugin-vue-libs and bump the ESLint major version in this repository.

Footnotes

  1. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Unicode_character_class_escape

@haoqunjiang haoqunjiang added the enhancement New feature or request label Jun 18, 2024
@dominikg
Copy link
Contributor Author

@sodatea your fix for this broke svelte-inspector for sveltekit +page files: sveltejs/vite-plugin-svelte#943 (comment)

is + an exploitable char in windows filenames?

@haoqunjiang
Copy link
Collaborator

After reading https://stackoverflow.com/q/4094699/2302258, I believe it's not exploitable.
Let me think this validation logic over. It might be better to escape than validate.

@dominikg
Copy link
Contributor Author

expanding the safe chars list or switching to only blocking bad chars both seem fine to me too, but if there is a way to properly escape the path before sending it to cmd thats even better

@oneezy
Copy link

oneezy commented Aug 24, 2024

this should be re-opened. we're still getting issues on Win11

Reference

issue: sveltejs/vite-plugin-svelte#943 (comment)

@Loizzus
Copy link

Loizzus commented Oct 8, 2024

Can confirm, issue has not been fixed at all for Svelte 4.
image

Also does not work at all in Svelte 5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants