php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #44037 dl() does'nt accept directories in module filename
Submitted: 2008-02-04 11:53 UTC Modified: 2009-05-10 15:00 UTC
Votes:20
Avg. Score:4.5 ± 1.2
Reproduced:16 of 16 (100.0%)
Same Version:11 (68.8%)
Same OS:7 (43.8%)
From: margus at zone dot ee Assigned:
Status: Wont fix Package: Feature/Change Request
PHP Version: 5.2.5 OS:
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: margus at zone dot ee
New email:
PHP Version: OS:

 

 [2008-02-04 11:53 UTC] 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


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2009-05-10 07:58 UTC] margus at zone dot ee
No response over a year.

Can staff from PHP team at least comment why the suggestion above is'nt considered yet?
 [2009-05-10 08:05 UTC] pajoye@php.net
dl is deprecated in many SAPIs. The extension_dir exists for this exact purpose.
 [2009-05-10 15:00 UTC] margus at zone dot ee
What has the deprecation to do with the issue at all?

According to docs, dl() is deprecated starting with PHP5.3 in all SAPI-s _except_ CLI, CGI and Embed. Also in PHP6 dl() is NOT deprecated in CLI,CGI and Embed.

I'll try to emphasize my points about dl() again:
- Lot's of users (only in my hosting it makes tens of thousands of users) are using PHP in CGI mode.
- dl() worked like ~10 years same way: it took extension_dir and merged it with module path and loaded the module. This was good for loading anykind of thirdparty modules like ionCube, phpShield, custom template engines etc. from custom directories, because you could specify relative path in the dl() parameter. Now you can't.
- It's very time consuming for administrator to install and maintain all users all custom modules in extension_dir. It's also not desired to have one user's private module loaded by another.
- It's very time and disk consuming to create separate extension_dir's for each user.
- It's NOT possible to change extension_dir during script to load one module from one dir and other module from another dir

Now suddenly an incompetent guy "fixes" an open_basedir flaw by breaking the old behaviour completely and this was done in between TWO maintenance versions (5.2.4 -> 5.2.5)

I would almost understand if the behaviour would be broken in minor version (like 5.2 vs 5.3)

Looks to me that PHP developers are quite ignorant and far away from the people who are actually using and making use of PHP...
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Dec 26 23:01:28 2024 UTC