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
possible parent path traversal in command argument #251
Comments
|
Sorry, I submitted before double-checking the repro steps and have to go afk. Will fix. |
|
@mikesamuel great catch. I don't believe we'll ever require nested command syntax so your second proposed solution should be great. Would you like to open a PR? If not, I don't mind, just let me know. |
|
I will prep a PR. |
mikesamuel
added a commit
to mikesamuel/entropic
that referenced
this issue
Jun 12, 2019
Fixes entropic-dev#251 This adds a check to function main so that when the first argument, which is interpreted as a command name, has a file separator, it is rejected, logged, and the command `help` is used instead. To ease testing, this splits the analysis code out of `function main` into `function unpack` which find the `cmd` to invoke but leaves the invoking and error recovery to `main`. This adds tests that a dodgy command is rejected, but that others like `whoami` continue to function. ```sh $ (pushd cli; npm t; popd) $ npm run lint ```
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is this a feature request or a bug?
bug
Parent path traversal is often considered a security problem.
This is likely low impact if it is a security problem; the only vectors I can see is socially engineered copy/paste or postinstall hook abuse.
If an attacker can get a JS file onto the machine, they might be able to use shell injection or malicious hooks to cause entropic to load that JS file, but an attacker can usually do anything via shell access that they could by running JS in the entropic process unless entropic evolved to accept multiple commands as a long-lived, privileged process.
Actual behavior:
entropic/cli/lib/main.js
Line 21 in ca58c5e
entropic ../../../node_modules/prettierfails with an error like "TypeError: ... is not a function"This works because
../../..backs out ofcli/lib/commandsto the root directory for entropic.Expected behavior:
A command passed as the zero-th argument at the command line should not cause line 21 to load a package outside the commands subdirectory. Line 21 should not load a dev dependency or a globally installed non-dependency.
Specifically,
entropic ../../../node_modules/prettiershould dump help info instead of potentially calling prettier's export as an async function.Steps to replicate:
Possible fix
One way to address is to define a
function isCommandArgumentthat usesrequire.resolveto convertargv[0]to a file path, then usepath.relativeto find a path relative to __DIRNAME and see if the first component of that relative path iscommands.Alternatively, if there is no desire to support commands like
subdir/basenamein the future then main could dump help text when/[\/\\]/.test(argv[0]).I can put together a PR if desired.
The text was updated successfully, but these errors were encountered: