Re: several messages (fwd)

[Marc Slemko <ma...@valis.worldgate.com>]

Once more...

sorry if multiple copies appear.


---------- Forwarded message ----------
Date: Mon, 13 Jan 1997 16:10:48 -0700 (MST)
From: Marc Slemko <ma...@valis.worldgate.com>
To: new-httpd@hyperreal.com
Subject: Re: several messages (fwd)

Does the list only accept posts from suybscribers?  

This didn't seem to go through the first time.  Trying again after
subscribing.

---------- Forwarded message ----------
Date: Mon, 13 Jan 1997 16:05:14 -0700 (MST)
From: Marc Slemko <ma...@valis.worldgate.com>
To: new-httpd@hyperreal.com
Subject: Re: several messages

First off, someone should update the web page to say "don't use 1.1.2 for
now, it broken".

I can't access my mail at home right now, so I'm just adding a few
comments... there are lots of different things below.  I'd appreciate it
if you could cc replies to this address. 

On Mon, 13 Jan 1997, sameer wrote:

> > I +1 these...
> > 
> > Since this is path-related, I can't see any real need not to do
> > so. Of course, we could also expand this to reduce '/./' -> '/'
> 
> 	The key is to understanding what kills stat()...
> 
> 	Does /././././././././.[etc] make stat fail?

If stat were passed a path like that which was too long, it would fail and
things would end up being bad.  However, that is stripped out before we
get to the stat call because it has a meaning in URLs; same thing as /../,
which is not necessarily the same meaning as in the underlying filesystem 
so it has to be dealt with earlier. 

`//` has no meaning in URLs, so it is left in.  

Most of the things that can cause problems on Unix filesystems do have
defined behavior WRT URLs so they are dealt with earlier.  But most is not
all.

On Mon, 13 Jan 1997, Ben Laurie wrote:

> Marc Slemko wrote:
> > best patch is the one below.  I don't like adding too many different
> > cases, because that reduces portability, but I am opposed to saying
> > "if it isn't ENOENT it must not exist at all".  Any better suggestions
> > are welcome... I don't really like the idea of stripping multiple
> > '/'s.
> 
> Why not? This solution is becoming increasingly diabolical. Stripping //s is
> simple and effective. If you are worried about overhead, just scan for //
> before stripping.
> 
> The problem was multiple slashes - the solution seems to have completely lost
> sight of the problem.

First, I have to say that this will not cause horrible things to happen
iff you only strip them for the stat as you are proposing.  I am only
mildly interested in the performance issues.  I am, however, very
concerned that it fixes the exploit but not the problem. 

My concern is that we should be stat()ing as close to the same filename we
try to open as possible.  The problem is NOT multiple slashes.  The
problem is that there is at least one condition where the name passed to
stat() can cause it to fail for whatever reason, but the path will still
be able to be read from. 

Suppose that there is an OS where `\` does the same thing that `/` does in
Unix.  Based on this, suppose that path makes it through the other layers
so it reaches the stat in question as "/foo/bar\\\\\\"....

Suppose that path is long enough to make stat() fail with ENAMETOOLONG. 
We will see the return code from stat and assume that it means the file
doesn't exist. Then later, because the OS ignores multiple `\`s, the
directory will be found just fine and return a directory and we have the
same exploit. 

My example above is fictional and I doubt that exact thing will work, but
it is the concept that is the problem.  Who are we to say that the only
reason stat can fail when the file exists is because of multiple '/'s?
The OS knows if it can stat() a path better than I do.

sameer writes:
>        Perhaps rather than checking for ENOENT and ENOTDIR, check for
>ENAMETOOLONG? (Or was that already discussed, and tossed for
>portability?)

I'm not sure portability is an issue, but it goes against basic security
principles.  Only allow what you know is good; don't just disallow what
you know is bad.

sameer writes:
>        which is why we should do ENOENT, ENOTDIR, -and- the
>/-squishage. I don't think we'll be able to think of every case.

IIf we are checking the errno from stat, why bother with squishing '/'s?

Someone else wrote:

>I think it musta been tossed for portability, I know it didn't work
>on linux.

Are you really sure?  Did you try it or just look at the include files?
Should work on everything back to kernel 1.0.  I did not notice any
response to my query for more info on that...