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 <[EMAIL PROTECTED]>]
>
>
>
>
> 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) {
>
>
>
