[security] Insecure usage of temporary files.
Closed this issue · 8 comments
The current code makes use of predictable filenames, in a way that causes a security issue.
I reported this to Debian last year:
It was recently highlighted by the nodesecurity people (six months later!):
Suggested fix:
- Avoid using predictable filenames in world-writable directories.
- Using
~/.app.pidwould be better than/tmpfor example.
- Using
The package node-cli insecurely uses the lock_file and log_file. Both of these are temporary, but it allows the starting user to overwrite any file they have access to.
I'm not entirely sure what the attack vector is here? If the "starting user" is running a command line node.js app, they can presumably already "overwrite any file they have access to" with <insert_coreutil>.
How would using ~ instead of /tmp mitigate this?
Consider a shared host with two logins:
evilcreates a symlink from/tmp/app.logpointing to/home/user/.bashrc.userruns an application that will create a predictable file in /tmp- Because that symlink exists it will be followed, with the net result that their
.bashrcfile is trashed.
- Because that symlink exists it will be followed, with the net result that their
Because both users can write to /tmp there is the potential for damage.
If, instead user only created ~/.app.log and ~/.app.pid then the evil user would not be able to write to it, to trigger the truncation/overwrite.
That makes sense, thanks.
Hi, thx for your nice Tool, but is there any Chance that this will be fixed soon? This marks other projects which use your outstanding tool as insecure on David batches, e.g. see https://david-dm.org/deadratfink/jy-transform/master.
Best, Jens
@chriso @skx: wouldn't using https://github.com/sindresorhus/tempfile be enough along with process.env.TEMP || process.env.TMP?
I'm proposing we resolve this by removing the functionality altogether. A little unorthodox, but you can see gh-86 for my rationale.
This is resolved in 1.0.0
Thanks, @chriso!