httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marc Slemko <ma...@znep.com>
Subject RE: IBM HTTP SERVER / APACHE (fwd)
Date Fri, 02 Jun 2000 03:50:02 GMT
On Thu, 1 Jun 2000, Allan Edwards wrote:

> As Ken said, I've been working on this and have a patch,
> I was making sure it held up before posting.
>  
> The bug is in ap_os_is_filename_valid (util_win32.c) 
> and the problem is that it checks for file path > MAX_PATH 
> (260 on NT), however stat fails for file paths >= MAX_PATH.

But shouldn't stat() return an error that can be checked to see if it is a
"good" failure (eg. file doesn't exist) or a bad one (eg. the cow is
jumping over the moon)?  This is done on Unix in several places, since
this isn't the only place where we have to know the difference between a
"it isn't there scotty" and a "I cannae read it capt'n".  An example is
reading .htaccess files.  While the path may well be canonicalized down
before it gets to that point on Windows, I think that checking the stat()
return codes is still a prudent thing to do.  We have run into a similar
or identical problem on Unix in the past, from what I recall.  But I
didn't look up the details, and my mind doesn't work so well right now.

I'm not saying your fix isn't valid, just that it seems there is a bigger
issue here.

Thanks for your work on this.  Who wants to post a quick note to bugtraq
describing the issue and presenting a temporary patch?  If no one else
wants to, I can, but otherwise someone involved would be better suited...

>  
> I've been able to reproduce the reported symptoms on 1.3 and
> and 2.0. Roy Marek's comments about variability from
> one server to another may be due to installation in different
> directories, it only fails for length 260.
>  
> How this translates to the reported bug: call sequence is-
> 
> handle_dir
> 	ap_subreq_lookupuri c:/apache/htdocs///...//index.html (260 bytes), 
> 		directory_walk
>             	get_path_info
>                   	ap_os_is_filename_valid  - returns OK
>                    	stat for ...///index.html fails, loop again
>                    	stat for c:/apache/htdocs succeeeds, 
> 					finfo.st_mode indicates director
>                   get_path_info sets st_mode = 0, returns OK
> 			ap_os_is_filename_valid returns OK
>             directory_walk returns status 200, st_mode=0
> 	      since st_mode=0 we do not internal redirect,
> 		pass to next handler mod_autoindex, BUG!!
>  
> With the patch below we get the following correct sequence:
> 
> handle_dir
> 	ap_subreq_lookupuri c:/apache/htdocs///...//index.html (260 bytes)
> 		directory_walk
> 			get_path_info
>                       	ap_os_is_filename_valid  - returns FAIL
>                       	loop again
>                       	stat for c:/apache/htdocs succeeeds, 
> 					finfo.st_mode indicates directory
> 			get_path_info sets st_mode = 0, returns OK
> 			ap_os_is_filename_valid returns HTTP_FORBIDDEN
>                   rnew->status = 403 is returned (same as paths > 260 bytes)
> 
> The Patch (against 1.3)
> ------------------------------------------------ 
> --- util_win32.c.org     Thu Jun 01 20:23:10 2000
> +++ util_win32.c    Thu Jun 01 20:23:38 2000
> @@ -580,7 +580,7 @@
>      };
> 
>      /* Test 1 */
> -    if (strlen(file) > MAX_PATH) {
> +    if (strlen(file) >= MAX_PATH) {
>      /* Path too long for Windows. Note that this test is not valid
>       * if the path starts with //?/ or \\?\. */
>      return 0;
> --------------------------------------------------
> 
> I'll apply this fix to 1.3 and 2.0 if no-one can see any holes in it.
> 
> Allan Edwards
> <ake@us.ibm.com>
> 


Mime
View raw message