Bug 36783 - request.c not correctly checking link owner uid for SymlinksIfOwnerMatch
Summary: request.c not correctly checking link owner uid for SymlinksIfOwnerMatch
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.1-HEAD
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: FixedInTrunk
: 29647 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-09-23 15:58 UTC by Robert L Mathews
Modified: 2011-09-17 15:47 UTC (History)
1 user (show)



Attachments
Patch for trunk (464 bytes, patch)
2005-09-23 16:01 UTC, Robert L Mathews
Details | Diff
Patch for 2.0.x (463 bytes, patch)
2005-09-23 16:03 UTC, Robert L Mathews
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert L Mathews 2005-09-23 15:58:54 UTC
The following code, around line 375 of server/request.c, contains an error  
that may lead to failures in SymlinksIfOwnerMatch on some platforms: 
    
  /* OPT_SYM_OWNER only works if we can get the owner of     
   * both the file and symlink.  First fill in a missing     
   * owner of the symlink, then get the info of the target.     
   */     
  if (!(lfi->valid & APR_FINFO_OWNER)) {     
      if ((res = apr_stat(&fi, d,      
                          lfi->valid | APR_FINFO_LINK | APR_FINFO_OWNER, p))     
          != APR_SUCCESS) {     
          return HTTP_FORBIDDEN;     
      }     
  }     
     
  if ((res = apr_stat(&fi, d, lfi->valid & ~(APR_FINFO_NAME), p))     
      != APR_SUCCESS) {     
      return HTTP_FORBIDDEN;     
  }     
     
  if (apr_uid_compare(fi.user, lfi->user) != APR_SUCCESS) {     
      return HTTP_FORBIDDEN;     
  }     
    
The apr_stat calls are supposed to set lfi->user and fi.user so they can be   
compared. However, they're both operating on &fi, meaning that lfi->user   
doesn't get set. 
     
On platforms where FINFO_OWNER isn't already valid when we reach this code 
(including Win32, according to William A. Rowe, Jr.), lfi->user could be 
random junk when compared, likely leading to a incorrect HTTP_FORBIDDEN result 
(and perhaps the small possibility of an incorrect OK result).  
    
The first apr_stat() call should set lfi instead of &fi:  
    
  if (!(lfi->valid & APR_FINFO_OWNER)) {     
      if ((res = apr_stat(lfi, d,
Comment 1 Robert L Mathews 2005-09-23 16:01:41 UTC
Created attachment 16500 [details]
Patch for trunk

This patch is for the version of request.c in the 2.2 trunk.
Comment 2 Robert L Mathews 2005-09-23 16:03:58 UTC
Created attachment 16501 [details]
Patch for 2.0.x

This patch is for the 2.0.x series.
Comment 3 Nick Kew 2008-03-02 22:37:46 UTC
Out of interest, what platform triggered this?  My testing never hits the code path you fixed, 'cos the test "if (!(lfi->valid & APR_FINFO_OWNER)) {" fails.

Anyway, fixed in trunk in r632947 - thanks.
Comment 4 Robert L Mathews 2008-03-04 11:53:07 UTC
(In reply to comment #3)
> Out of interest, what platform triggered this?  My testing never hits the code
> path you fixed, 'cos the test "if (!(lfi->valid & APR_FINFO_OWNER)) {" fails.

The bug doesn't happen on my platform (Linux); I caught it just by looking at the code while making an unrelated change to how FollowSymLinks works on our copy of Apache.

But according to this post, it might happen on Win32:

http://mail-archives.apache.org/mod_mbox/httpd-dev/200509.mbox/<43339114.3060705%40rowe-clan.net>


> Anyway, fixed in trunk in r632947

Thanks!
Comment 5 Nick Kew 2010-07-20 09:41:44 UTC
*** Bug 29647 has been marked as a duplicate of this bug. ***
Comment 6 Stefan Fritsch 2011-09-17 15:47:34 UTC
2.2.17 / r989124