php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #10322 Logical error in fopen-wrappers.c
Submitted: 2001-04-13 18:52 UTC Modified: 2005-01-31 23:04 UTC
From: php-bugrep at pgregg dot com Assigned:
Status: Not a bug Package: Safe Mode/open_basedir
PHP Version: 4.0.4pl1 OS: All
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: php-bugrep at pgregg dot com
New email:
PHP Version: OS:

 

 [2001-04-13 18:52 UTC] php-bugrep at pgregg dot com
In main/fopen_wrappers.c I see that there is a function:
PHPAPI int php_check_specific_open_basedir(char *basedir, char *path PLS_DC)

However "basedir" is never used in this function at all,
only PG(open_basedir).

Surely this negates the point of the function being called individually for each tokenised entry on open_basedir/php.ini?   I think it will only match, ever, on the first entry in the config file.

So, should all references to PG(open_basedir) in php_check_specific_open_basedir() be replaced with the arg basedir ?

Thanks,

Paul Gregg
(Hacking in support for DOCUMENT_ROOT as another "specical case")

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2001-04-13 20:57 UTC] php-bugrep at pgregg dot com
I thought, while I'm here, I'd submit a patch to fix this.

The patch also includes support for an additional special case in php.ini's open_basedir.
The current "." allows scripts to access files in the same directory as the script.
"DOCUMENT_ROOT" allows a script to access any other file in the virtualhost's directory tree.  DOCUMENT_ROOT is calculated by PATH_TRANSLATED and removing SCRIPT_URI from the end - This conveniently works for both full Apache Virtalhosts and mod_aliased Mass virtual hosting (I don't know if this is true for the newer mod_vhost - just check what PATH_TRANSLATED and SCRIPT_URI is set to in phpinfo() - if removing the latter from the former is the sites docroot then you are away).

Anyway, the patch: code shamelessly copied from the "." segment :)

*** main/fopen-wrappers.c.orig  Fri Apr 13 17:50:02 2001
--- main/fopen-wrappers.c       Sat Apr 14 01:46:28 2001
***************
*** 141,151 ****
        char resolved_name[MAXPATHLEN];
        char resolved_basedir[MAXPATHLEN];
        char local_open_basedir[MAXPATHLEN];
        int local_open_basedir_pos;
        SLS_FETCH();

        /* Special case basedir==".": Use script-directory */
!       if ((strcmp(PG(open_basedir), ".") == 0) &&
                SG(request_info).path_translated &&
                *SG(request_info).path_translated
                ) {
--- 141,167 ----
        char resolved_name[MAXPATHLEN];
        char resolved_basedir[MAXPATHLEN];
        char local_open_basedir[MAXPATHLEN];
+       char *local_open_request_uri;
        int local_open_basedir_pos;
        SLS_FETCH();

+       /* Special case basedir="DOCUMENT_ROOT": Restrict to directory of the
+        * virtualhost itself as calculated by PATH_TRANSLATED - SCRIPT_URI
+        * php@pgregg.com
+        */
+       if ((strcmp(basedir, "DOCUMENT_ROOT") == 0) &&
+               SG(request_info).path_translated &&
+               *SG(request_info).path_translated ) {
+               /* Copy path_translated to local_open_basedir, the look in
+                  this string for where request_uri starts and zero that byte
+                  thus leaving local_open_basedir set to the virtualhost's
+                  DOCUMENT_ROOT */
+               strlcpy(local_open_basedir, SG(request_info).path_translated, si
zeof(local_open_basedir));
+               local_open_request_uri=strstr(local_open_basedir,SG(request_info
).request_uri);
+               if (local_open_request_uri) *local_open_request_uri = '\0';
+       } else
        /* Special case basedir==".": Use script-directory */
!       if ((strcmp(basedir, ".") == 0) &&
                SG(request_info).path_translated &&
                *SG(request_info).path_translated
                ) {


(I realise cut-n-paste into this window will convert all the nice TABS to spaces, so a patch < this file isn't going to be as clean).

I'd really appreciate this being included in the next patchlevel / release as I'd guess that anyone already using "." will want this.

Paul.
 [2001-04-14 05:34 UTC] jmoore@php.net
This will not make it into 4.0.5 as this was branched a while back but it might well make it into 4.0.6. Ill get a developer to look at this patch.

- James
 [2001-04-14 08:20 UTC] php-bugrep at pgregg dot com
Thanks for the response, however, can you confirm that there is a logical coding error here?

The DOCUMENT_ROOT patch is secondary and would be nice - but is the bug going to be fixed in 4.0.5 ?

Regards,

Paul.
 [2001-04-16 11:12 UTC] jason@php.net
This is almost an exact copy of a patch I had submitted in October of 2000.(before I became a contributor).
http://marc.theaimsgroup.com/?l=php-dev&m=97145490702792&w=2
This idea (and many others) was on hold to a cleaner redesign of safe_mode.

There is NO logical error, basedir is used, check the patch that YOU *supposedly* submited, it uses basedir. 
Please don't try and claim other peoples code as your own.

Marked as Bogus

-Jason

 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Fri Jul 19 23:01:25 2019 UTC