Re: writing files securely

[Colin Phipps <cph@netcraft.com>]

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/