0

When you autoload classes with either __autoload or spl_autoload_register do you have to sanitize the data if the variable used to initialize the class is dynamically initialized using explode($_SERVER['REQUEST_URI'])?

Example:

$arr=explode($_SERVER['REQUEST_URI'],DIRECTORY_SEPARATOR);
$obj=new $arr[0];

__autoload

define('PATH',dirname(__FILE__).DIRECTORY_SEPARATOR);
function __autoload($class){if(file_exists(PATH.'library/'.$class.'.php');include(PATH.'library/'.$class.'.php');}

spl_autoload_register

define('PATH',dirname(__FILE__).DIRECTORY_SEPARATOR);
function myautoloader($class){if(file_exists(PATH.'library/'.$class.'.php');include(PATH.'library/'.$class.'.php');}
spl_autoload_register('myautoloader');

to my knowledge if you attempt site.com/../../../etc/passwd%00 it would simply give a 404 because there is no GET parameter ?get=

Every thing I stated is most likely untrue on Windows Servers because the path is different ..\..\..\ but is this correct if you talk about a Linux/BSD server? If not, why not?

KaekeaSchmear
  • 1,548
  • 5
  • 18
  • 30
  • 1
    `__autoload` receives the class name as parameter and you do what you must to declare it. There are no additional parameters envolved, doesn't matter how you tried to use the class and where, its just as simply as it gets. I'm not sure what you are thinking. – Havenard Apr 08 '14 at 02:32
  • 2
    Note that `__autoload` will not instance the object, it's job is to declare the class. It is only called if a class you just tried to use isn't declared anywhere. – Havenard Apr 08 '14 at 02:33
  • @Havenard I'm asking whether the parameter has to be sanitized in case file inclusion exploits in the situation I just drawn out. – KaekeaSchmear Apr 08 '14 at 02:34
  • Class names just as variables and functions can only be named with a limited range of symbols, so I don't think you have to worry about that. They are implicitly sanitized. – Havenard Apr 08 '14 at 02:36
  • @Havenard So, you are saying that even if you have code such as http://pastebin.com/epGHTZee and there is a `config.php` in the parent directory the following injection would not work `?get=../config`? – KaekeaSchmear Apr 08 '14 at 02:42
  • 1
    Just test it. You cannot declare a `new ../config`, its a synthax error. – Havenard Apr 08 '14 at 02:47
  • But even if its sanitized I don't recommend you do it. Are you thinking of allowing the user to instance classes in your code? It can be a serious mistake. – Havenard Apr 08 '14 at 02:48
  • @Havenard Yes, however it still includes `config.php` before the `fatal error` which is a local file inclusion vulnerability. Well, i'm trying to writing a frontcontroller and want my `app` to handle requests like shown in the post. – KaekeaSchmear Apr 08 '14 at 02:49
  • Ok, then here is what you should do. Have an array with a list of valid class names the user is allowed to instance. Check if the class name he is trying to instance exists in that array with `in_array`, if not, don't proceed. You absolutely should not allow the user to instance any kind of class he names, sanitized or not. – Havenard Apr 08 '14 at 02:51
  • @Havenard Nonetheless, I would like to know if in the situation I sketched this vulnerability still exists. That was my question. – KaekeaSchmear Apr 08 '14 at 02:54
  • I'm not sure, it might depend on unpredictable factors. Better just make it the right way and not only prevent a vulnerability but also prevent unhandled errors. – Havenard Apr 08 '14 at 02:56

0 Answers0