httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: cvs commit: httpd-2.0/server request.c
Date Fri, 15 Mar 2002 14:17:45 GMT
Jeff,

   I like this solution as well! [sorry, I've had little time on my hands,
and no time to chime in on your earlier pleas for comment :-( ]

   There are two other cases; the pre-queried or quick-stat'ed
r->finfo before we entered dir_walk's path parsing loop, and the
the initial lstat() leg, which we don't hit with this patch.  [e.g.
only an APR_LNK that is re-stat()'ed is caught in this leg
of the code.]  Do we need extra tests on those two legs,
as well?  E.g. subreq_lookup_file may already have stat()ed
the file, or [per the group's request] we will try to stat() on
entry and save ourselves the effort within the loop.

   I don't know that I would be opposed to catching this in
apr_stat() as well.  Sure, apr_file_open would succeed in
spite of the fact that apr_stat() fails, but the point to the
apr_file and apr_filepath functions were to normalize many
different OS'es into similar behavior.  You wouldn't need to
actually stat() a file to reject from apr_open(), either.  Simply
testing for a trailing slash would be sufficient.  But I'd rather
see us keep our 'faith' low right now, and have extra tests
in dir_walk that ensure security, then to discover exploits
later on.  dir_walk was the right [first] patch :-)

Thanks for all your research on this odd exception!

Bill





At 07:35 AM 3/15/2002, you wrote:
>trawick     02/03/15 05:35:42
>
>   Modified:    .        CHANGES
>                server   request.c
>   Log:
>   Allow URIs specifying CGI scripts to include '/' at the end
>   (e.g., /cgi-bin/printenv/) on AIX and Solaris (and other OSs
>   which ignore '/' at the end of the names of non-directories).
>
>   PR:    10138
>
>   Revision  Changes    Path
>   1.636     +5 -0      httpd-2.0/CHANGES
>
>   Index: CHANGES
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/CHANGES,v
>   retrieving revision 1.635
>   retrieving revision 1.636
>   diff -u -r1.635 -r1.636
>   --- CHANGES   14 Mar 2002 23:31:22 -0000      1.635
>   +++ CHANGES   15 Mar 2002 13:35:42 -0000      1.636
>   @@ -1,5 +1,10 @@
>    Changes with Apache 2.0.34-dev
>
>   +  *) Allow URIs specifying CGI scripts to include '/' at the end
>   +     (e.g., /cgi-bin/printenv/) on AIX and Solaris (and other OSs
>   +     which ignore '/' at the end of the names of non-directories).
>   +     PR 10138  [Jeff Trawick]
>   +
>      *) implement SSLSessionCache shmht and shmcb based on apr_rmm and
>         apr_shm.  [Madhusudan Mathihalli <madhusudan_mathihalli@hp.com>]
>
>
>
>
>   1.107     +14 -0     httpd-2.0/server/request.c
>
>   Index: request.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/request.c,v
>   retrieving revision 1.106
>   retrieving revision 1.107
>   diff -u -r1.106 -r1.107
>   --- request.c 13 Mar 2002 20:48:00 -0000      1.106
>   +++ request.c 15 Mar 2002 13:35:42 -0000      1.107
>   @@ -556,6 +556,20 @@
>         */
>        if (!r->finfo.filetype || r->finfo.filetype == APR_LNK) {
>            apr_stat(&r->finfo, r->filename, APR_FINFO_MIN, r->pool);
>   +
>   +        /* some OSs will return APR_SUCCESS/APR_REG if we stat
>   +         * a regular file but we have '/' at the end of the name;
>   +         *
>   +         * other OSs will return APR_ENOTDIR for that situation;
>   +         *
>   +         * handle it the same everywhere by simulating a failure
>   +         * if it looks like a directory but really isn't
>   +         */
>   +        if (r->finfo.filetype &&
>   +            r->finfo.filetype != APR_DIR &&
>   +            r->filename[strlen(r->filename) - 1] == '/') {
>   +            r->finfo.filetype = 0; /* forget what we learned */
>   +        }
>        }
>
>        if (r->finfo.filetype == APR_REG) {
>
>
>



Mime
View raw message