Assigning to [~bkarwin] to initiate issue review.

Changing to 'Unassigned'

Adding this somewhere before {{$path}} is defined in {{Zend_Loader::loadClass()}} would work:

{{$class = preg_replace('/[a-zA-Z0-9_\x7f-\xff]/', '', $class);}}

Patch:

Index: /library/Zend/Loader.php
===================================================================
--- /library/Zend/Loader.php    (revision 7100)
+++ /library/Zend/Loader.php    (working copy)
@@ -66,6 +66,8 @@
             $dirs = (array) $dirs;
         }
 
+        // remove invalid class name characters
+        $class = preg_replace('/[a-zA-Z0-9_\x7f-\xff]/', '', $class);
         // autodiscover the path from the class name
         $path = str_replace('_', DIRECTORY_SEPARATOR, $class);
         if ($path != $class) {

loadClass() uses the _securityCheck() method internally which already performs these checks.

I have tried this code $className = 'Mypackage_Filter_'.ucfirst(strtolower($_GET['module'])); try{ Zend_Loader::loadClass($className); }catch(Zend_Exception $e){ echo $e->getMessage(); }

and when passed something like module=../../otherfile in the url, it will automatically include the file. It seems that the _securitiCheck() is not correctly applied.

The security check is for filenames, not for classes. loadClass() normalizes the class name to an appropriate file name; securityCheck() simply checks that no invalid filesystem characters are used in the path provided, and both '.' and '/' are valid characters.

That said, '.' and '/' are not valid characters for class names, and PHP will fail the class_exists() check within loadClass(), raising an exception.

Hi Mathew, Actually the class_exists and interface_exists will not fail. I've just run the debugger on that . It all comes to passing the wrong file name to _securityCheck function

$classNameAfterloadClassFunction = 'Mypackage\Filter\../../../checkpoint.php';

if (preg_match('/[^a-z0-9\\/\\\\_.-]/i', $classNameAfterloadClassFunction)) {
            throw new Zend_Exception('Security check: Illegal character in filename');
}else{
    echo "no exception";
}
die();

I'm on win xp sp2 / php 5.24 / apache 2.2.4.

Okay, let me run this by you. I have the following:

$className = 'Zend_Acl/../../../../../../tmp/foo';
Zend_Loader::loadClass($className);

In side foo.php, I have the following:

echo "This got executed\n";

If I execute the original script, I get the following:

This got executed
PHP Fatal error:  Uncaught exception 'Zend_Exception' with message 'File "Zend/Acl/../../../../../../tmp/foo.php" does not exist or class "Zend_Acl/../../../../../../tmp/foo" was not found in the file' in /home/matthew/git/framework-standard/trunk/library/Zend/Loader.php:88
Stack trace:
#0 /home/matthew/tmp/test.php(18): Zend_Loader::loadClass('Zend_Acl/../../...')
#1 {main}
  thrown in /home/matthew/git/framework-standard/trunk/library/Zend/Loader.php on line 88

Zend_Exception: File "Zend/Acl/../../../../../../tmp/foo.php" does not exist or class "Zend_Acl/../../../../../../tmp/foo" was not found in the file in /home/matthew/git/framework-standard/trunk/library/Zend/Loader.php on line 88

Call Stack:
    0.0002      51000   1. {main}() /home/matthew/tmp/test.php:0
    0.0009      98644   2. Zend_Loader::loadClass() /home/matthew/tmp/test.php:18

If I change foo.php to create the following:

class Zend_Acl/../../../../../../tmp/foo
{
}

I instead get this:

PHP Parse error:  syntax error, unexpected '/', expecting '{' in /home/matthew/tmp/foo.php on line 2
PHP Stack trace:
PHP   1. {main}() /home/matthew/tmp/test.php:0
PHP   2. Zend_Loader::loadClass() /home/matthew/tmp/test.php:18

Parse error: syntax error, unexpected '/', expecting '{' in /home/matthew/tmp/foo.php on line 2

Call Stack:
    0.0002      51000   1. {main}() /home/matthew/tmp/test.php:0
    0.0009      98644   2. Zend_Loader::loadClass() /home/matthew/tmp/test.php:18

What I'm driving at here is that loadClass() is going to generate either an exception because the class does not exist, or a parse error because invalid characters were used to define the class name.

What YOU are trying to get across is that Zend_Loader::loadClass() allows a user to include an arbitrary file. This will be true if you use include/require or their *_once variants by themselves as it is. The question is whether or not we should add a check for invalid class name characters or path separators prior to calling the include. While I can see value in this, it also adds significant overhead to Zend_Loader, particularly as we already have the _securityCheck() method running as well. (If you don't believe this adds up, try running a typical ZF app through a debugger sometime.)

_securityCheck() is for determining valid filesystem characters -- removing '/' or '.' from that would make it basically unusable. The only way to prevent this is to add an additional check to the class name -- which, as noted, would be more overhead.

I'm going to reopen the issue and mark it as postponed, as I need to discuss it with a few others prior to making a decision.

You completely described my issue :). The file I was using (checkpoint.php) generates an rss feed, so the error was added to the generated file but not displayed by Firefox (only viewable in view source).