Skip to content

ext/soap: Fix integer overflow when decoding SOAP array indexes#21963

Open
LamentXU123 wants to merge 4 commits intophp:masterfrom
LamentXU123:sec-fix
Open

ext/soap: Fix integer overflow when decoding SOAP array indexes#21963
LamentXU123 wants to merge 4 commits intophp:masterfrom
LamentXU123:sec-fix

Conversation

@LamentXU123
Copy link
Copy Markdown
Contributor

SOAP encoded array metadata such as arrayType, offset, and position is parsed into int indexes before being used for array placement. That is, this code:

<?php
class C extends SoapClient {
    function __doRequest($r, $l, $a, $v, $o = false, ?string $u = null): string {
        return <<<'XML'
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/"
 xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/"
 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 xmlns:xsd="http://www.w3.org/2001/XMLSchema"
 SOAP-ENV:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
 <SOAP-ENV:Body>
  <x:testResponse xmlns:x="x">
   <return SOAP-ENC:arrayType="xsd:string[1]" SOAP-ENC:offset="[2147483648]" xsi:type="SOAP-ENC:Array">
    <item xsi:type="xsd:string">x</item>
   </return>
  </x:testResponse>
 </SOAP-ENV:Body>
</SOAP-ENV:Envelope>
XML;
    }
}

$c = new C(null, ['location' => 'x', 'uri' => 'x']);
var_dump($c->test());

Will resulted in:

array(1) {
  [-2147483648]=>
  string(1) "x"
}

Because the index overflows.

There is a TODO message to fix this. So I think people are aware of this bug before

I therefore wrote this to add checked decimal accumulation for SOAP array indexes and reject values that exceed int range. I Also reject auto-increment past INT_MAX before updating the next array position :)

@LamentXU123 LamentXU123 requested a review from devnexen as a code owner May 6, 2026 14:54
@LamentXU123 LamentXU123 changed the title ext/soap: Fix SOAP array index integer overflow ext/soap: Fix integer overflow when decoding SOAP array indexes May 6, 2026
@LamentXU123
Copy link
Copy Markdown
Contributor Author

LamentXU123 commented May 6, 2026

Plus: Had to fix the bailout logic....

Now in the client side, if I raise a error when overflow is detected, the code will go to master_to_eval() and without xmlFreeDoc(response) and cause a memory leak. So in case of that I just had to add some logic to the bailout process to make sure xmldoc is free, although I hate it (;′⌒`)

Comment thread ext/soap/php_encoding.c
if (pos[i] >= dims[i]) {
if (i > 0) {
pos[i] = 0;
} else {
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.

kind of empty now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants