[BusyBox] Bug in busybox sed -i

Rob Landley rob at landley.net
Fri May 13 23:48:19 UTC 2005


Doug Swarin pointed out a security bug in the -i option of sed.

While the permissions on the temp file are correct to prevent it from being 
maliciously mangled  by passing strangers, (created with 600, opened O_EXCL, 
etc), the permissions on the _directory_ might not be, and we re-open the 
file to convert the filehandle to a FILE * (and automatically get an error 
message and exit if the directory's read-only or out of space or some such).

This opens a potential race condition if somebody's using dnotify on the 
directory, deletes/renames the tempfile, and drops a symlink or something 
there.  Somebody running sed -i as root in a world readable directory could 
do damage.

I dug up notes on an earlier discussion where we looked at the security 
implications of this (unfortunately on the #uclibc channel rather than email; 
I don't have a transcript, just notes-to-self) which pointed out that if the 
permissions on the directory allow other people's files to be deleted/renamed 
then the original file is vulnerable to sabotage anyway.  However, there are 
two cases that discussion apparently didn't take into account:

1) Using another user's permissions to damage files in other directories you 
can't access (standard symlink attack).

2) Reading data another user couldn't otherwise access by having the new file 
belong to that other user.

I've attached a proposed fix: use fdopen on the open file handle and do the 
error checking ourselves.  Doug, this patch is against
http://www.busybox.net/cgi-bin/viewcvs.cgi/trunk/busybox/editors/sed.c?rev=10121&view=log

Comments?

Rob.

P.S.  Yeah, there are a few other fixes in my tree polluting this patch.  Some 
sie reductions -- switch an error exit to use the libbb function and zap a 
duplicate call to atexit().  And Bernhard's recent "static" annotation passed 
put a "static" on a pure type declaration, which makes no sense and is 
causing a warning.  Now the functionality pass has calmed down I should 
probably do a size pass, but that's _after_ the SuSv3 audit...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sed.patch
Type: text/x-diff
Size: 1422 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20050513/a4759f61/attachment.bin 


More information about the busybox mailing list