Re: lynx-dev 2.8.1dev.19.patch.gz

[Bela Lubkin]

T.E.Dickey wrote:

> 1998-07-31 (2.8.1dev.19)

Briefly, for the impatient: dev.19 re-introduces serious /tmp security
holes -- don't use it unless you are using per-user Lynx temp dirs.

> * add option -eat_all_cookies and corresponding config variable
>   EAT_ALL_COOKIES (Brian J Pardy <address@hidden>).

I thought it was agreed that the word "eat" was too confusing.  This
should be "-accept_all_cookies", no?

> * correct spurious 'Content' string versus newline after X-URL in LYMain.c
>   (Bela Lubkin and Larry Virden).

LYMail.c

> * modify OpenHiddenFile so that it can overwrite files owned by the real
>   user if the O_EXCL open fails because the file already exists - TD

This modification reintroduces temp file problems!  Part of the point of
opening with O_CREAT | O_EXCL is that this combination does not follow
symbolic links.  If you strictly use O_CREAT | O_EXCL (and other related
precautions, and are generally very careful), it is safe to use a public
/tmp directory as long as the directory is "sticky" (doesn't allow users
to delete other users' files).

The new code takes that away again.  I can create a symlink to one of
your files.  The first open(O_CREAT | O_EXCL) will fail with EEXIST, so
you'll go into the new code and truncate the file.

Suppose I know root's about to run Lynx.  I can anticipate one of the
temp filenames it's going to use, and do:

  ln -s /.rhosts /tmp/$ANTICIPATED_NAME

I can easily trick root into trashing any file on the system, or at
least any file that is actually owned by root.  By being sufficiently
devious, I'm sure I can also break into root.  Or I can have any other
user trash his own files, and possibly break into his account.

Yes, it's generally safer for Lynx to put its temp files into a separate
user-owned directory for each user.  But few systems are actually
configured that way.  We should not be re-introducing security holes for
systems configured in the most normal way!

Any solution which involves safety checks is probably unsafe.  I'm
talking about pseudo-code like:

  1. use stat() to see whether the file currently exists
  2. then use open() to create it

    -- has a race condition where an attacker can create the dangerous
       symlink between steps 1 and 2

  1. open(O_CREAT | O_EXCL)
  2. if failed with EEXIST, open(O_CREAT)

    -- not even a race condition, can be attacked trivially

  1. open(O_CREAT | O_EXCL)
  2. if failed with EEXIST,
  3.   use stat() to verify it's a real file, not a symlink; and that
       it's actually owned by us
  4.   then open(O_CREAT)

    -- can be attacked trivially for files on the same filesystem as the
       directory (use hard links instead of symlinks)

What was the impetus for this change?  When Lynx creates these temporary
files, it should be using a pseudo-random filename.  If the exclusive
create attempt fails with EEXIST, it should be moving on to the next
name in its pseudo-random sequence.  Note that the randomness is not a
security feature; it isn't intended to make it "impossible" for an
attacker to anticipate the name.  It is to increase the likelihood of
finding a usable filename in a small number of attempts.  Using a
serialized sequence opens the possibility of having to search through
hundreds of names before finding a good one.  A properly designed random
sequence will only have that problem if almost its entire name space is
in use.

>Bela<