Home » Web Services » XML_RPC » Bug #4775

[2005-07-09 14:13 UTC] giunta dot gaetano at sea-aeroportimilano dot it

Description:
------------
If received xml is bogus, the eval could fail, leading to $v not being an xmlrpcval object. But xml_rpc_response creator called later on has no provision for this.
All that is missing is to check if $allOk == 1 (and to set it to 0 immediately before). See phpxmlrpc ver 1.1 for the complete fix...
The same logic should be applied to the eval used to build params in server.php.

Note that this fixes Bug #3344, too: we should NOT allow malformed xml received over the net to 'break' the code.

Reproduce code:
---------------
See code used for discovery of security breach 1 week ago

[2005-07-11 10:41 UTC] giunta dot gaetano at sea-aeroportimilano dot it

The bug (or whatever you call it) is found in rel 1.3.2, which I had just checked out.

The patch is the following:

$r = new XML_RPC_Response(0, $XML_RPC_err['invalid_return'],
$XML_RPC_str['invalid_return']);
} else {
$allOK=0;
@eval('$v=' . $XML_RPC_xh[$parser]['st'] . '; $allOK=1;');
if ($XML_RPC_xh[$parser]['isf']) {
$f = $v->structmem('faultCode');
$fs = $v->structmem('faultString');
$r = new XML_RPC_Response($v, $f->scalarval(),
$fs->scalarval());
} else {
if ($allOK)
$r = new XML_RPC_Response(0, XML_RPC_ERROR_INVALID_RESPONSE, ''XML_RPC_ERROR_INVALID_RESPONSE');
else
$r = new XML_RPC_Response($v);
}

Bye

[2005-07-18 13:41 UTC] giunta dot gaetano at sea-aeroportimilano dot it

Here are some bits of XML that I tested against the latest CVS version of phpxmlrpc, some as request payload, some as response payload.

They are all wrong, in that they do not follow the xmlrpc spec, but are still valid xml.

The expected result is that the lib should not generate PHP warnings / errors when parsing them and 1- pass on to userspace well-constructed xmlrp_obj ojects or 2- respond back to caller with well defined xmlrpc error responses.

<?xml version="1.0"?>
<methodCall>
<methodName>system.methodHelp</methodName>
<param>
<value><string>system.methodHelp</string></value>
</param>
</methodCall>

<?xml version="1.0"?>
<methodCall>
<methodName>system.methodHelp</methodName>
<params>
<value><string>system.methodHelp</string></value>
</params>
</methodCall>

<?xml version="1.0"?>
<methodCall>
<methodName>system.methodHelp</methodName>
<params>
<param><string>system.methodHelp</string></param>
</params>
</methodCall>

<?xml version="1.0"?>
<methodResponse>
<param>
<value><string>system.methodHelp</string></value>
</param>
</methodResponse>

<?xml version="1.0"?>
<methodResponse>
<params>
<value><string>system.methodHelp</string></value>
</params>
</methodResponse>

<?xml version="1.0"?>
<methodResponse>
<params>
<param><string>system.methodHelp</string></param>
</params>
</methodResponse>

PS: in phpxmlrpc I modified the xmlrpc_msg::AddParam() method: it checks for the passed param to be a valid xmlrpcval obj first, and returns true on success and false on error. This helps to determine if something went wrong when the servers builds the xmlrpcmsg object to be passed to userspace.