Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ PHP NEWS
- OpenSSL:
. Fix compatibility issues with OpenSSL 4.0. (jordikroon, Remi)

- SOAP:
. Fixed integer overflow when decoding SOAP array indexes. (Weilin Du)

- SPL:
. Fix SplFixedArray::setSize leak when destructor grows during clear.
(David Carlier)
Expand Down
38 changes: 26 additions & 12 deletions ext/soap/php_encoding.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
+----------------------------------------------------------------------+
*/

#include <limits.h>
#include <time.h>

#include "php_soap.h"
Expand Down Expand Up @@ -2075,6 +2076,15 @@ static int calc_dimension_12(const char* str)
return i;
}

static void soap_array_position_add_digit(int *position, int digit)
{
if (*position > (INT_MAX - digit) / 10) {
soap_error0(E_ERROR, "Encoding: array index out of range");
}

*position = (*position * 10) + digit;
}

static int* get_position_12(int dimension, const char* str)
{
int *pos;
Expand All @@ -2095,7 +2105,7 @@ static int* get_position_12(int dimension, const char* str)
i++;
flag = 1;
}
pos[i] = (pos[i]*10)+(*str-'0');
soap_array_position_add_digit(&pos[i], *str - '0');
} else if (*str == '*') {
soap_error0(E_ERROR, "Encoding: '*' may only be first arraySize value in list");
} else {
Expand Down Expand Up @@ -2125,7 +2135,7 @@ static void get_position_ex(int dimension, const char* str, int** pos)
memset(*pos,0,sizeof(int)*dimension);
while (*str != ']' && *str != '\0' && i < dimension) {
if (*str >= '0' && *str <= '9') {
(*pos)[i] = ((*pos)[i]*10)+(*str-'0');
soap_array_position_add_digit(&(*pos)[i], *str - '0');
} else if (*str == ',') {
i++;
}
Expand Down Expand Up @@ -2698,16 +2708,20 @@ static zval *to_zval_array(zval *ret, encodeTypePtr type, xmlNodePtr data)
/* Increment position */
i = dimension;
while (i > 0) {
i--;
pos[i]++;
if (pos[i] >= dims[i]) {
if (i > 0) {
pos[i] = 0;
} else {
/* TODO: Array index overflow */
}
} else {
break;
i--;
if (pos[i] == INT_MAX) {
efree(dims);
efree(pos);
zval_ptr_dtor(ret);
ZVAL_UNDEF(ret);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed ; soap_error0(E_ERROR, …) should always bail through the SOAP error handler in practice, but leaving ret pointing to freed array data on the off-chance the
bailout path ever changes is a footgun we don't need. The ZVAL_UNDEF keeps the returned zval safe regardless.

soap_error0(E_ERROR, "Encoding: array index out of range");
}
pos[i]++;
if (pos[i] < dims[i]) {
break;
}
if (i > 0) {
pos[i] = 0;
}
}
}
Expand Down
40 changes: 31 additions & 9 deletions ext/soap/php_packet_soap.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,28 @@

#include "php_soap.h"

static void master_to_zval_with_doc_cleanup(zval *ret, encodePtr encode, xmlNodePtr data, xmlDocPtr doc)
{
bool bailout = false;

ZVAL_UNDEF(ret);

/* SoapClient can turn decode errors into a bailout before parse_packet_soap() frees the response doc. */
zend_try {
master_to_zval(ret, encode, data);
} zend_catch {
bailout = true;
} zend_end_try();

if (bailout) {
if (!Z_ISUNDEF_P(ret)) {
zval_ptr_dtor(ret);
}
xmlFreeDoc(doc);
zend_bailout();
}
}

/* SOAP client calls this function to parse response from SOAP server */
bool parse_packet_soap(zval *this_ptr, char *buffer, int buffer_size, sdlFunctionPtr fn, char *fn_name, zval *return_value, zval *soap_headers)
{
Expand Down Expand Up @@ -191,22 +213,22 @@ bool parse_packet_soap(zval *this_ptr, char *buffer, int buffer_size, sdlFunctio
tmp = get_node(fault->children, "faultstring");
if (tmp != NULL && tmp->children != NULL) {
zval zv;
master_to_zval(&zv, get_conversion(IS_STRING), tmp);
master_to_zval_with_doc_cleanup(&zv, get_conversion(IS_STRING), tmp, response);
convert_to_string(&zv)
faultstring = Z_STR(zv);
}

tmp = get_node(fault->children, "faultactor");
if (tmp != NULL && tmp->children != NULL) {
zval zv;
master_to_zval(&zv, get_conversion(IS_STRING), tmp);
master_to_zval_with_doc_cleanup(&zv, get_conversion(IS_STRING), tmp, response);
convert_to_string(&zv)
faultactor = Z_STR(zv);
}

tmp = get_node(fault->children, "detail");
if (tmp != NULL) {
master_to_zval(&details, NULL, tmp);
master_to_zval_with_doc_cleanup(&details, NULL, tmp, response);
}
} else {
tmp = get_node(fault->children, "Code");
Expand All @@ -223,15 +245,15 @@ bool parse_packet_soap(zval *this_ptr, char *buffer, int buffer_size, sdlFunctio
tmp = get_node(tmp->children,"Text");
if (tmp != NULL && tmp->children != NULL) {
zval zv;
master_to_zval(&zv, get_conversion(IS_STRING), tmp);
master_to_zval_with_doc_cleanup(&zv, get_conversion(IS_STRING), tmp, response);
convert_to_string(&zv)
faultstring = Z_STR(zv);
}
}

tmp = get_node(fault->children,"Detail");
if (tmp != NULL) {
master_to_zval(&details, NULL, tmp);
master_to_zval_with_doc_cleanup(&details, NULL, tmp, response);
}
}
add_soap_fault(this_ptr, faultcode, faultstring ? ZSTR_VAL(faultstring) : NULL, faultactor ? ZSTR_VAL(faultactor) : NULL, &details);
Expand Down Expand Up @@ -324,9 +346,9 @@ bool parse_packet_soap(zval *this_ptr, char *buffer, int buffer_size, sdlFunctio
} else {
/* Decoding value of parameter */
if (param != NULL) {
master_to_zval(&tmp, param->encode, val);
master_to_zval_with_doc_cleanup(&tmp, param->encode, val, response);
} else {
master_to_zval(&tmp, NULL, val);
master_to_zval_with_doc_cleanup(&tmp, NULL, val, response);
}
}
add_assoc_zval(return_value, param->paramName, &tmp);
Expand All @@ -347,7 +369,7 @@ bool parse_packet_soap(zval *this_ptr, char *buffer, int buffer_size, sdlFunctio
zval tmp;
zval *arr;

master_to_zval(&tmp, NULL, val);
master_to_zval_with_doc_cleanup(&tmp, NULL, val, response);
if (val->name) {
if ((arr = zend_hash_str_find(Z_ARRVAL_P(return_value), (char*)val->name, strlen((char*)val->name))) != NULL) {
add_next_index_zval(arr, &tmp);
Expand Down Expand Up @@ -412,7 +434,7 @@ bool parse_packet_soap(zval *this_ptr, char *buffer, int buffer_size, sdlFunctio
}
smart_str_free(&key);
}
master_to_zval(&val, enc, trav);
master_to_zval_with_doc_cleanup(&val, enc, trav, response);
add_assoc_zval(soap_headers, (char*)trav->name, &val);
}
trav = trav->next;
Expand Down
24 changes: 22 additions & 2 deletions ext/soap/soap.c
Original file line number Diff line number Diff line change
Expand Up @@ -2408,9 +2408,19 @@ static void do_soap_call(zend_execute_data *execute_data,
request = NULL;

if (ret && Z_TYPE(response) == IS_STRING) {
bool parse_bailout = false;

encode_reset_ns();
ret = parse_packet_soap(this_ptr, Z_STRVAL(response), Z_STRLEN(response), fn, NULL, return_value, output_headers);
zend_try {
ret = parse_packet_soap(this_ptr, Z_STRVAL(response), Z_STRLEN(response), fn, NULL, return_value, output_headers);
} zend_catch {
parse_bailout = true;
} zend_end_try();
encode_finish();
if (parse_bailout) {
zval_ptr_dtor(&response);
zend_bailout();
}
}

zval_ptr_dtor(&response);
Expand Down Expand Up @@ -2452,9 +2462,19 @@ static void do_soap_call(zend_execute_data *execute_data,
request = NULL;

if (ret && Z_TYPE(response) == IS_STRING) {
bool parse_bailout = false;

encode_reset_ns();
ret = parse_packet_soap(this_ptr, Z_STRVAL(response), Z_STRLEN(response), NULL, NULL, return_value, output_headers);
zend_try {
ret = parse_packet_soap(this_ptr, Z_STRVAL(response), Z_STRLEN(response), NULL, NULL, return_value, output_headers);
} zend_catch {
parse_bailout = true;
} zend_end_try();
encode_finish();
if (parse_bailout) {
zval_ptr_dtor(&response);
zend_bailout();
}
}

zval_ptr_dtor(&response);
Expand Down
89 changes: 89 additions & 0 deletions ext/soap/tests/soap_array_index_overflow.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
--TEST--
SOAP array index overflow is rejected
--EXTENSIONS--
soap
--FILE--
<?php
class TestSoapClient extends SoapClient {
public string $response;

public function __doRequest($request, $location, $action, $version, $one_way = false, ?string $uriParserClass = null): string {
return $this->response;
}
}

function soap_response(string $attributes, string $itemAttributes = ''): string {
return <<<XML
<?xml version="1.0" encoding="UTF-8"?>
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/"
xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/"
xmlns:xsd="http://www.w3.org/2001/XMLSchema"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:ns1="http://example.org/"
SOAP-ENV:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
<SOAP-ENV:Body>
<ns1:testResponse>
<return $attributes>
<item xsi:type="xsd:string" $itemAttributes>value</item>
</return>
</ns1:testResponse>
</SOAP-ENV:Body>
</SOAP-ENV:Envelope>
XML;
}

function test_overflow(string $name, string $response): void {
$client = new TestSoapClient(NULL, [
'location' => 'test://',
'uri' => 'http://example.org/',
'exceptions' => true,
]);
$client->response = $response;

try {
$client->test();
echo "$name: no fault\n";
} catch (SoapFault $e) {
echo "$name: $e->faultstring\n";
}
}

function test_boundary_position(): void {
$client = new TestSoapClient(NULL, [
'location' => 'test://',
'uri' => 'http://example.org/',
'exceptions' => true,
]);
$client->response = soap_response(
'SOAP-ENC:arrayType="xsd:string[1]" xsi:type="SOAP-ENC:Array"',
'SOAP-ENC:position="[2147483646]"'
);

var_dump($client->test());
}

test_overflow(
'arrayType',
soap_response('SOAP-ENC:arrayType="xsd:string[2147483648]" xsi:type="SOAP-ENC:Array"')
);

test_overflow(
'offset',
soap_response('SOAP-ENC:arrayType="xsd:string[1]" SOAP-ENC:offset="[2147483648]" xsi:type="SOAP-ENC:Array"')
);

test_overflow(
'position',
soap_response('SOAP-ENC:arrayType="xsd:string[1]" xsi:type="SOAP-ENC:Array"', 'SOAP-ENC:position="[2147483647]"')
);

test_boundary_position();
?>
--EXPECT--
arrayType: SOAP-ERROR: Encoding: array index out of range
offset: SOAP-ERROR: Encoding: array index out of range
position: SOAP-ERROR: Encoding: array index out of range
array(1) {
[2147483646]=>
string(5) "value"
}
Loading