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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
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

Pull Requests

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-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 28 10:01:29 2024 UTC