From cca9385fbc2ead4bcb0d13bc046cca0d5fd094b5 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Wed, 11 Oct 2023 00:11:16 +0200
Subject: [PATCH] Fix GH-12392: Segmentation fault on SoapClient::__getTypes

There are two issues:
- UAF because the hashmap resized while being iterated over, yet the local
  variables used internally in the macros are not updated.
- The hashmap being iterated over is modified: entries are deleted after
  other entries have been added. This causes the deletion to fail sometimes
  because indices of buckets have shifted.

Fix it by using a while loop iteration and HashPosition position tracker
instead.
Issue exists on PHP 8.1 too, but is much harder to trigger.
The test file reproduces the issue reliably on PHP 8.2 and up.
---
 ext/soap/php_schema.c       | 18 +++++++----
 ext/soap/tests/gh12392.phpt | 28 +++++++++++++++++
 ext/soap/tests/gh12392.wsdl | 61 +++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 6 deletions(-)
 create mode 100644 ext/soap/tests/gh12392.phpt
 create mode 100644 ext/soap/tests/gh12392.wsdl

diff --git a/ext/soap/php_schema.c b/ext/soap/php_schema.c
index e93679a55ea39..521c5d0f1d9ed 100644
--- a/ext/soap/php_schema.c
+++ b/ext/soap/php_schema.c
@@ -2261,17 +2261,23 @@ static void schema_type_fixup(sdlCtx *ctx, sdlTypePtr type)
 		schema_content_model_fixup(ctx, type->model);
 	}
 	if (type->attributes) {
-		zend_string *str_key;
-		zend_ulong index;
+		HashPosition pos;
+		zend_hash_internal_pointer_reset_ex(type->attributes, &pos);
 
-		ZEND_HASH_FOREACH_KEY_PTR(type->attributes, index, str_key, attr) {
-			if (str_key) {
+		while ((attr = zend_hash_get_current_data_ptr_ex(type->attributes, &pos)) != NULL) {
+			zend_string *str_key;
+			zend_ulong index;
+
+			if (zend_hash_get_current_key_ex(type->attributes, &str_key, &index, &pos) == HASH_KEY_IS_STRING) {
 				schema_attribute_fixup(ctx, attr);
+				zend_result result = zend_hash_move_forward_ex(type->attributes, &pos);
+				ZEND_ASSERT(result == SUCCESS);
 			} else {
 				schema_attributegroup_fixup(ctx, attr, type->attributes);
-				zend_hash_index_del(type->attributes, index);
+				zend_result result = zend_hash_index_del(type->attributes, index);
+				ZEND_ASSERT(result == SUCCESS);
 			}
-		} ZEND_HASH_FOREACH_END();
+		}
 	}
 }
 
diff --git a/ext/soap/tests/gh12392.phpt b/ext/soap/tests/gh12392.phpt
new file mode 100644
index 0000000000000..8a234ba025be9
--- /dev/null
+++ b/ext/soap/tests/gh12392.phpt
@@ -0,0 +1,28 @@
+--TEST--
+GH-12392 (Segmentation fault on SoapClient::__getTypes)
+--EXTENSIONS--
+soap
+--FILE--
+<?php
+
+$client = new SoapClient(__DIR__ . "/gh12392.wsdl", ['cache_wsdl' => WSDL_CACHE_NONE]);
+echo 'Client created!' . "\n";
+
+$types = $client->__getTypes();
+echo 'Got types!' . "\n";
+
+var_dump($types);
+
+?>
+--EXPECT--
+Client created!
+Got types!
+array(1) {
+  [0]=>
+  string(62) "struct dummy {
+ string foo;
+ string a;
+ string b;
+ string c;
+}"
+}
diff --git a/ext/soap/tests/gh12392.wsdl b/ext/soap/tests/gh12392.wsdl
new file mode 100644
index 0000000000000..5e9a7ea094fb8
--- /dev/null
+++ b/ext/soap/tests/gh12392.wsdl
@@ -0,0 +1,61 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<wsdl:definitions
+  xmlns:xs="http://www.w3.org/2001/XMLSchema"
+  xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/"
+  xmlns="http://schemas.xmlsoap.org/wsdl/"
+  xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/"
+  targetNamespace="http://xoev.de/schemata/xzufi/2_2_0">
+  <wsdl:types>
+    <xs:schema xmlns:ns="http://php.net" targetNamespace="http://php.net">
+      <xs:attributeGroup name="c">
+        <xs:attribute name="c" type="string" />
+      </xs:attributeGroup>
+
+      <xs:attributeGroup name="b">
+        <xs:attribute name="b" type="string" />
+      </xs:attributeGroup>
+
+      <xs:attributeGroup name="a">
+        <xs:attribute name="a" type="string" />
+        <xs:attributeGroup ref="ns:b" />
+      </xs:attributeGroup>
+
+      <xs:complexType name="dummy">
+        <xs:sequence>
+          <xs:element name="foo" type="string" />
+        </xs:sequence>
+        <xs:attributeGroup ref="ns:a" />
+        <xs:attributeGroup ref="ns:c" />
+      </xs:complexType>
+    </xs:schema>
+  </wsdl:types>
+
+  <!-- Below is a shortened copy of the test.wsdl, doesn't matter, only the types matter -->
+
+  <message name="AddRequest">
+    <part name="x" type="xs:double" />
+  </message>
+
+  <portType name="TestServicePortType">
+    <operation name="Add">
+      <input message="tns:AddRequest" />
+    </operation>
+  </portType>
+
+  <binding name="TestServiceBinding" type="tns:TestServicePortType">
+    <soap:binding style="rpc" transport="http://schemas.xmlsoap.org/soap/http" />
+    <operation name="Add">
+      <soap:operation soapAction="Add" style="rpc" />
+      <input>
+        <soap:body use="encoded" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" />
+      </input>
+    </operation>
+  </binding>
+
+  <service name="TestService">
+    <port name="TestServicePort" binding="tns:TestServiceBinding">
+      <soap:address location="http://linuxsrv.home/~dmitry/soap/soap_server.php" />
+    </port>
+  </service>
+
+</wsdl:definitions>