https://sourceware.org/bugzilla/show_bug.cgi?id=26945
--- Comment #13 from Rich Felker <bugdal at aerifal dot cx> --- This patch keeps expanding way beyond the scope of what I can commit to review, and I don't think it's nearing something that fixes the problem. It's not clear to me if bfd_stat uses fstat on the open file (safe) or stat on the name (unsafe) and I could go look this up, but this is ballooning into something much larger than I can take on and after I look that up there will be something else and something else. You really need someone who understands both this code and temp file/writable-dir TOCTOU race vulnerabilities to work with you on the fix. Short of that, the only thing I can say with confidence is that the chmod/chown behavior should be removed entirely (and even then I'm not sure it'd be safe to run in directories other users can write to, but at least it wouldn't explicitly be offering a feature meant only for that use case and encouraging users to use it that way). On major bug I will comment on individually -- in smart_rename you added a line: to_fd = open (to, O_WRONLY | O_TRUNC | O_BINARY, 0777); I don't understand the motivation for that at all, but it's creating a *completely new vulnerability* of the type we're discussing here. (And even when it's not being maliciously exploited, it will cause data loss whenever the file is a hardlink!) It looks like you're trying to avoid calling stat on the target because I told you that's unsafe, but instead you're truncating the target, which might be a symlink or hardlink to any file on the whole filesystem, *then* checking, after truncating it, whether it has any hardlinks. Here you really do want to stick with lstat. You're not using the result to copy permissions, just to see if the target you're about to replace would be a hard link or symlink, in which case I think the idea is to overwrite the existing file rather than replacing it so the linked status is preserved (at the cost of atomicity). There inherently will be races where you do "the wrong thing" here, and opening the file first doesn't do anything to avoid that. But they should not be "exploitable" for anything beyond annoyance. -- You are receiving this mail because: You are on the CC list for the bug.
