sifive / wake

The SiFive wake build tool
Other
86 stars 28 forks source link

Overwriting a hardlinked file greatly broadens permissions #1593

Open ag-eitilt opened 2 months ago

ag-eitilt commented 2 months ago

Split from #1589: If a Wake job overwrites a file which has been hardlinked to elsewhere, the known output is delinked and overwritten as expected. However, the remaining copy of the inode suddenly gains 777 permissions.

$ wake -x 'makePlan "test" Nil "echo -n test > test.txt" | runJob' ; ln test.txt test2.txt ; chmod -w test2.txt ; wake -x 'makePlan "test" Nil "echo -n test with different contents > test.txt" | runJob'
Job 9666
Job 9679
$ ls -l test.txt test2.txt
-rwxrwxrwx 1 sammay sammay  4 Jul  8 12:48 test2.txt
-rw-r--r-- 1 sammay sammay 28 Jul  8 12:48 test.txt

Our guess during the meeting is that this is a problem with our FUSE sandboxing, likely somewhere in deep_unlink and specifically if the file isn't writable, trying to deinitialize the inode without realizing that it's still accessible. So far as we can tell this doesn't provide a vector for permission escalation attacks as the Linux permission system would block any attempt to change permissions on files which the user running Wake wouldn't already have access to, but it is technically possible to construct a script which calls ln [...]/target.file . a victim into a Wake workspace and deliberately overwrite that to expose the original -- it's simply also possible to chmod 777 [...]/target.file directly.

Our determination is that this is definitely a problem which needs to be fixed, but isn't a critical problem to drop everything for. It also points to the fact that we likely shouldn't be rolling our own FUSE implementation, and that switching that whole subsystem out for overlayfs would eliminate a whole category of potential bugs lurking in edge cases such as this.