[PATCH] D93881: [llvm-objcopy] preserve file ownership when overwritten

[Jian Cai via Phabricator via llvm-commits]

jcai19 added a comment.



> GNU objcopy before 2.36 had the symlink attach vulnerability. The patch would introduce the similar vulnerability to llvm-objcopy.

@MaskRay, I am not familiar with this vulnerability, so just to clarify, the vulnerability  in my code can be exploited by a symlink attack after it opens the file but before it calls fchown, i.e. (1) llvm-objcopy opens the output file (2) attackers created a symlink with the same name, (3) fchown would change the ownership of the symlink instead of the intended file. Is that correct?

> But I think we should do better: instead of: (1) create a temp file (2) open it by name again (3) check it is a regular file with one hard link & it is writable (S_IWUSR) (4) fstat (5) fchown,
> we should just modify the destination in place (for objcopy file and strip file). If the file has been mapped readonly, on many platforms it may be unwritable. We have to accept this.

Can we use absolute path instead of relative path to shield the code from symlink attacks? This should also avoid the above-mentioned issue on readonly files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93881/new/

https://reviews.llvm.org/D93881