On Mon, Apr 30, 2001 at 03:47:03PM +0200, Josip Rodin wrote:
> There was a problem with joe editor overwriting files with a symlink attack
> using the DEADJOE file.
[snip]
> I'm currently reviewing a new version of joe that uses patches from other
> sources, and this is the chunk of code they used to fix this issue:
>
> [...]
> long tim = time (0);
> B *b;
> FILE *f;
> int tmpfd;
> struct stat sbuf;
>
> if ((tmpfd = open ("DEADJOE", O_RDWR | O_EXCL | O_CREAT, 0600)) < 0)
> {
> if (lstat ("DEADJOE", &sbuf) < 0)
> _exit (-1);
> if (!S_ISREG (sbuf.st_mode) || sbuf.st_uid != geteuid ())
> _exit (-1);
That won't catch attacks using hard links.
> /*
> A race condition still exists between the lstat() and the
> open()
> systemcall, which leads to a possible denial-of-service
> attack
> by setting the file access mode to 600 for every file the
> user executing joe has permissions to.
> This can't be fixed w/o breacking the behavior of the
> orig. joe!
> */
The comment puts it well.
> if ((tmpfd = open ("DEADJOE", O_RDWR | O_APPEND)) < 0)
> _exit (-1);
> if (fchmod (tmpfd, S_IRUSR | S_IWUSR) < 0)
> _exit (-1);
The chmod is not going to help, the attacker will just open the file
beforehand.
[snip]
> I'm inclined to think Wichert's fix is better
I'll second that.
--
Colin Phipps http://www.netcraft.com/
