httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ken Parzygnat" <>
Subject RE: [PATCH] Win32 device files
Date Thu, 03 Dec 1998 18:05:29 GMT
> Below is a patch to fix a security problem when accessing Windows 32
> device files. The problem is if a requrest is made for a URI such as /aux.
> This is mapped onto a special DOS device file called AUX (no matter which
> directory is the document root). This can cause problems. There are other
> DOS device files as well, which at worse can cause a lock-up of the
> server.
> The patch adds a new OS abstraction: ap_os_is_filename_valid(). This is
> given a path and returns 0 or 1 depending on whether the path name looks
> valid or not, according to various rules.
> This function is called from three places: 
>   First, whenever we are going to open a configuration file with
>   pcfg_openfile. This guards against references to configuration
>   files with invalid filenames.
>   Secondly, in get_path_info when trying to decide how much of a request
>   URI is the real path and how much is the PATH_INFO. This is to avoid
>   stat()'ing an invalid filename. (Note that get_path_info() works
>   backwards from the complete path, so we cannot reject invalid filenames
>   at this stage because the invalid part may be in the PATH_INFO).
>   Thirdly, after the real filename has been found in directory_walk()
>   to avoid trying to actually open an invalid filename when returning
>   a response. We return HTTP_FORBIDDEN if we have an invalid filename
>   at this point.
> This patch should help fix the security hole, but has a few drawbacks: it
> is slow, it does not cover all situations when we open files (e.g. you can
> still specify PidFile /LPT1), and it will reject use of NUL in
> AccessConfig or ResourceConfig directives. It will also reject invalid
> path components which may be on a remote share where the path component is
> infact valid.

Cool.  I'm really glad you created the new function instead of putting this
logic in the os_canonical_filename stuff!!  It should be easier to manage.

I've played around with the patch and things are looking good, but I did find
two things:

If I create an alias like:
Alias /croot/ c:/

The patch will make this fail, while the pre-patched code works fine.  I suspect
the culprit is test #3.  I seem to remember a place or two in the code that will
append a '.' to names like x:/  

I wonder if we need test #3 in ap_os_is_filename_valid since 
ap_os_case_canonical_filename silently removes the trailing '.'?

Also, an alias like:
Alias /sharedir/ //machine/share/

Fails with the patch.  It seems that there may be a problem
with the .htaccess processing, though I haven't tracked that down.

- - - - - - - - - - - - - - - - - -
Ken Parzygnat

View raw message