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

[LamentXU123]

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]

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 (；′⌒`)

[devnexen]

looks good, but I ll have another look more toward the end of this week (and likely merging it in the process). Thanks for working on it !

[devnexen]

as mentioned, overall good just few minor nits to address !

[LamentXU123]

as mentioned, overall good just few minor nits to address !

Thanks for reviewing! The fix should be in place in a couple of minutes.

Now I think this is rather a bug fix so it is more proper to rebase this on 8.4 I don't sure if this should be counted as a minor security fix(signed integer overflow) if than this should go to 8.2

[devnexen]

I m not sure it would qualify as actual security fix for 8.2 but let me discuss it internally

[LamentXU123]

I m not sure it would qualify as actual security fix for 8.2 but let me discuss it internally

Take your time :) I am first going to rebase this on 8.4

[LamentXU123]

Okay, now its on 8.4 and I squash every commit on this branch into one commit so it would be easier to merge.

[devnexen]

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.

[devnexen]

technically correct, LGTM if CI green.

[LamentXU123]

technically correct, LGTM if CI green.

Feel free to merge after the targeted branch is decided :)