ID:          44037
 Comment by:  margus at zone dot ee
 Reported By: margus at zone dot ee
 Status:      Open
 Bug Type:    Feature/Change Request
 PHP Version: 5.2.5
 New Comment:

No response over a year.

Can staff from PHP team at least comment why the suggestion above is'nt
considered yet?


Previous Comments:
------------------------------------------------------------------------

[2008-02-04 11:53:46] margus at zone dot ee

Description:
------------
When full directory path is given to dl() (which contain slashes), then
error message "Temporary module name should contain only filename in
..." is displayed.

This brutal change in dl() behaviour has been introduced starting with
PHP 5.2.5 and is supposed to be an security fix for open_basedir bypass.
The fix actually does'nt check module filename against open_basedir at
all but rejects all files which contain slash in their name.

(Original report for this can be found here:
http://securityreason.com/securityalert/3119)

NB! Full directory name is neccessary when module is'nt located in
global extension_dir.

I think it would be more logical and flexible if there was an explicit
open_basedir check (like in other file operation functions) instead of
current slash-checking-hack which suddenly breaks lots of applications
which load modules like ionCube, phpShield etc. in runtime.

In shared environments (like virtualhosting) it's common for users to
have their specific modules in their own home directories which of
course fall outside of global extension_dir.

This is the proposal for correct open_basedir check:

--- dl.c.original       2008-02-04 13:35:15.000000000 +0200
+++ dl.c        2008-02-04 13:36:43.000000000 +0200
@@ -130,13 +130,6 @@
        if (extension_dir && extension_dir[0]){
                int extension_dir_len = strlen(extension_dir);
 
-               if (type == MODULE_TEMPORARY) {
-                       if (strchr(Z_STRVAL_P(file), '/') != NULL ||
strchr(Z_STRVAL_P(file), DEFAULT_SLASH) != NULL) {
-                               php_error_docref(NULL TSRMLS_CC,
E_WARNING, "Temporary module name should contain only filename");
-                               RETURN_FALSE;
-                       }
-               }
-
                if (IS_SLASH(extension_dir[extension_dir_len-1])) {
                        spprintf(&libpath, 0, "%s%s", extension_dir,
Z_STRVAL_P(file));
                } else {
@@ -146,6 +139,10 @@
                libpath = estrndup(Z_STRVAL_P(file),
Z_STRLEN_P(file));
        }
 
+       if (php_check_open_basedir(libpath TSRMLS_CC)) {
+               RETURN_FALSE;
+       }
+
        /* load dynamic symbol */
        handle = DL_LOAD(libpath);
        if (!handle) {

Reproduce code:
---------------
php.ini:
extension_dir=/usr/local/lib/php/extensions/no-debug-non-zts-20060613
open_basedir=.:/home/myuser/

test.php:
<?php
if (dl('/../../../../../../home/myuser/somemodule.so')) {
   echo "Module loaded";
}
?>

Expected result:
----------------
Module loaded!

Actual result:
--------------
Warning: dl(): Temporary module name should contain only filename in
test.php on line 2



------------------------------------------------------------------------


-- 
Edit this bug report at http://bugs.php.net/?id=44037&edit=1

Reply via email to