security issue in DemoCompUpdate component

[carsten888]

In the download on this page
http://docs.joomla.org/Managing_Compone ... 6_-_Part_1
the var 'controller' can be manipulated to include onther files in democompupdate.php.

If someone was to take this demo code as a base for an extension, it would be vunerability waiting to be abused.

[Chris Davenport]

Thanks for the report. Please feel free to correct it.

Chris.

[carsten888]

Code: Select all

// Require specific controller if requested
if ( $controller = JRequest::getVar( 'controller' ) )
{
    $path = JPATH_COMPONENT.DS.'controllers'.DS.$controller.'.php';
    if ( file_exists( $path ) ) { require_once $path; }
    else { $controller = ''; }
}

// Create the controller
$classname    = 'DemocompupdateController'.ucfirst($controller);
$controller   = new $classname( );

replace with

Code: Select all

$controller   = new DemocompupdateController( );

[ashwani1489]

Hello carsten888,
I am new to joomla, and eager to know how it is a vulnerability. The code you have suggested is not substitute of what is written above. What to do if we use your code in controller and want to use multiple controllers for diffrent purpose.

thanks

[ashwani1489]

I used the same code while i make any custom component. so i want to know how it is a vulnerability so that i may also make a secure custom component in future.

thanks

[carsten888]

@ashwani1489
yes, that code does not do the same. This component only has 1 controller, so that is the savest way to do that.
For how to deal with more then one controller read the tut here:
http://docs.joomla.org/Developing_a_Mod ... _-_Part_02
o, shoot, I just noticed that is not in the tut.

Code: Select all

// Require specific controller if requested
if ( $controller = JRequest::getVar( 'controller' ) )
{
    $path = JPATH_COMPONENT.DS.'controllers'.DS.$controller.'.php';
    if ( file_exists( $path ) ) { require_once $path; }
    else { $controller = ''; }
}

// Create the controller
$classname    = 'DemocompupdateController'.ucfirst($controller);
$controller   = new $classname( );

replace with:

Code: Select all

// Require specific controller if requested
if ( $controller = JRequest::getVar( 'controller' ) )
{
    $path = JPATH_COMPONENT.DS.'controllers'.DS.$controller.'.php';
	$allowed_controllers = array('items', 'categories');//specify here all allowed controllers
    if ( file_exists( $path ) && in_array($controller, $allowed_controllers)) { require_once $path; }
    else { $controller = ''; }
}

// Create the controller
$classname    = 'DemocompupdateController'.ucfirst($controller);
$controller   = new $classname( );

[ashwani1489]

@ashwani1489
yes, that code does not do the same. This component only has 1 controller, so that is the savest way to do that.
For how to deal with more then one controller read the tut here:
http://docs.joomla.org/Developing_a_Mod ... _-_Part_02
o, shoot, I just noticed that is not in the tut.

Code: Select all

// Require specific controller if requested
if ( $controller = JRequest::getVar( 'controller' ) )
{
    $path = JPATH_COMPONENT.DS.'controllers'.DS.$controller.'.php';
    if ( file_exists( $path ) ) { require_once $path; }
    else { $controller = ''; }
}

// Create the controller
$classname    = 'DemocompupdateController'.ucfirst($controller);
$controller   = new $classname( );

replace with:

Code: Select all

// Require specific controller if requested
if ( $controller = JRequest::getVar( 'controller' ) )
{
    $path = JPATH_COMPONENT.DS.'controllers'.DS.$controller.'.php';
	$allowed_controllers = array('items', 'categories');//specify here all allowed controllers
    if ( file_exists( $path ) && in_array($controller, $allowed_controllers)) { require_once $path; }
    else { $controller = ''; }
}

// Create the controller
$classname    = 'DemocompupdateController'.ucfirst($controller);
$controller   = new $classname( );

thanks for your nice explaination