I have e up with the following bits of code to call a method via AJAX in my PHP classes:
PHP:
class Ajax extends Controller {
private $class;
private $method;
private $params;
function __construct()
{
$this->params = $_POST; // Call params
$call = explode('->', $this->params['call']);
$this->class = new $call[0]; // e.g. controller->method
$this->method = $call[1];
array_shift($this->params);
$this->parse();
}
public function index()
{
//Dummy
}
public function parse()
{
$r = '';
$r = call_user_func_array(array($this->class, $this->method), $this->params);
echo $r;
}
}
Client:
function creditCheck2(id)
{
$.post(ROOT + 'Ajax', {call: 'Record->creditState', id: id, enquiryid: enquiryId}, function(data) {
alert(data)
}, 'json')
}
It seems to work great, but is it secure and could it be better?
Just for reference, I have added my code with the changes suggested by the answers:
class Call extends Controller {
private $class;
private $method;
private $params;
private $authClasses = array(
'Gallery'
);
function __construct()
{
$this->params = $_POST; // Call params
$call = explode('->', $this->params['call']);
if(!in_array($call[0], $this->authClasses))
{
die();
}
$this->class = new $call[0]; // e.g. controller->method
$this->method = $call[1];
unset($this->params['call']);
$this->parse();
}
public function parse()
{
$r = '';
$param = array();
// Params in any order...
$mRef = new ReflectionMethod($this->class, $this->method);
foreach($mRef->getParameters() as $p) {
$param[$p->name] = $this->params[$p->name];
}
$this->params = $param;
if($r = @call_user_func_array(array($this->class, $this->method), $this->params))
{
echo $r;
}
else {
}
}
}
I have e up with the following bits of code to call a method via AJAX in my PHP classes:
PHP:
class Ajax extends Controller {
private $class;
private $method;
private $params;
function __construct()
{
$this->params = $_POST; // Call params
$call = explode('->', $this->params['call']);
$this->class = new $call[0]; // e.g. controller->method
$this->method = $call[1];
array_shift($this->params);
$this->parse();
}
public function index()
{
//Dummy
}
public function parse()
{
$r = '';
$r = call_user_func_array(array($this->class, $this->method), $this->params);
echo $r;
}
}
Client:
function creditCheck2(id)
{
$.post(ROOT + 'Ajax', {call: 'Record->creditState', id: id, enquiryid: enquiryId}, function(data) {
alert(data)
}, 'json')
}
It seems to work great, but is it secure and could it be better?
Just for reference, I have added my code with the changes suggested by the answers:
class Call extends Controller {
private $class;
private $method;
private $params;
private $authClasses = array(
'Gallery'
);
function __construct()
{
$this->params = $_POST; // Call params
$call = explode('->', $this->params['call']);
if(!in_array($call[0], $this->authClasses))
{
die();
}
$this->class = new $call[0]; // e.g. controller->method
$this->method = $call[1];
unset($this->params['call']);
$this->parse();
}
public function parse()
{
$r = '';
$param = array();
// Params in any order...
$mRef = new ReflectionMethod($this->class, $this->method);
foreach($mRef->getParameters() as $p) {
$param[$p->name] = $this->params[$p->name];
}
$this->params = $param;
if($r = @call_user_func_array(array($this->class, $this->method), $this->params))
{
echo $r;
}
else {
}
}
}
It could be better in that array_shift($this->params) unnecessarily assumes that the first item in the params array will always be call. That's not true and it does not agree with the direct access $this->params['call'] you are doing a little earlier. The array_shift should be replaced with simply unset($this->params['call']).
There is also the problem that the order of values in the params array must match the order of parameters in the signature of the method you are trying to call. I don't think there is a guarantee that the order will be the same as the order of the parameters in the AJAX request, so that's a theoretical problem.
More importantly, this way of doing things forces the author of the AJAX code to match the order of parameters in the signature of the method you are trying to call. This introduces a horrible level of coupling and is a major problem. What's worse, changing the order of the parameters by mistake will not be apparent. Consider:
public function bankTransfer($fromAccount, $toAccount, $amount);
$.post(ROOT + 'Ajax', {
call: 'Bank->bankTransfer',
from: "sender",
to: "recipient",
amount: 42
}, function(data) { ... });
This would work. But if you do this
$.post(ROOT + 'Ajax', {
call: 'Bank->bankTransfer',
to: "recipient", // swapped the order of
from: "sender", // these two lines
amount: 42
}, function(data) { ... });
You will get the opposite result of what is expected. I believe it's immediately obvious that this is extremely bad.
To solve the problem you would have to use reflection to match the array keys in $this->params with the formal names of the parameters of the method being called.
Finally, this code is insecure in that anyone can make a request that directs your code to call any method of any class with the appropriate parameters -- even methods that should not be accessible from a web environment.
This is another serious problem and cannot really be fixed unless you introduce some type of filtering to the dispatch logic.
It seems to work great, but is it secure and could it be better?
Are you using your own framework or using other framework? I believe that it isn't secure at all, if the attacker know what might be inside your framework. For example: there is database class in your framework, attacker can do the following:
{call: 'Database->execute', sql: 'SELECT * FROM information_schema.`tables`'}
You can limit the number of class that you allow user to access. For example:
if (!in_array($this->class, array('Record', 'Hello'))) {
die();
}
This is sample of reflection that I just learn (Thanks to @Jon for the reference). This solve the problem of passing argument in different order from the PHP function.
class Email
{
public function send($from, $to, $msg) {
return "Send $from to $to: $msg";
}
}
$rawParam = array('msg' => 'Hello World',
'to' => '[email protected]',
'from' => '[email protected]');
$param = array();
// Rearrange
$methodRef = new ReflectionMethod('Email', 'send');
foreach($methodRef->getParameters() as $p) {
$param[$p->name] = $rawParam[$p->name];
}
var_dump($rawParam);
var_dump($param);