From 97bb48ec2ed5af0992383ae02b4251f1a0c769ad Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen@gmail.com>
Date: Sat, 14 Mar 2026 19:00:22 +0000
Subject: [PATCH] ext/ffi: Fix resource leak in FFI::cdef() on symbol
 resolution failure.

and remove duplicate conditions in read/write field handlers.
close GH-21446
---
 NEWS                                          |  4 ++
 ext/ffi/ffi.c                                 | 44 ++++++++++---------
 ext/ffi/tests/gh14286_2.phpt                  |  4 --
 ext/ffi/tests/gh18629_cdef_resolve_func.phpt  | 16 +++++++
 .../tests/gh18629_read_field_non_struct.phpt  | 17 +++++++
 5 files changed, 60 insertions(+), 25 deletions(-)
 create mode 100644 ext/ffi/tests/gh18629_cdef_resolve_func.phpt
 create mode 100644 ext/ffi/tests/gh18629_read_field_non_struct.phpt

diff --git a/NEWS b/NEWS
index e10a9d68d49..a7797641886 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-21486 (Dom\HTMLDocument parser mangles xml:space and
     xml:lang attributes). (ndossche)
 
+- FFI:
+  . Fixed resource leak in FFI::cdef() onsymbol resolution failure.
+    (David Carlier)
+
 - GD:
   . Fixed bug GH-21431 (phpinfo() to display libJPEG 10.0 support).
     (David Carlier)
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 5fdacc6d0eb..6fb00a330d8 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -1257,10 +1257,8 @@ static zval *zend_ffi_cdata_read_field(zend_object *obj, zend_string *field_name
 			type = ZEND_FFI_TYPE(type->pointer.type);
 		}
 		if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
-			if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
-				zend_throw_error(zend_ffi_exception_ce, "Attempt to read field '%s' of non C struct/union", ZSTR_VAL(field_name));
-				return &EG(uninitialized_zval);
-			}
+			zend_throw_error(zend_ffi_exception_ce, "Attempt to read field '%s' of non C struct/union", ZSTR_VAL(field_name));
+			return &EG(uninitialized_zval);
 		}
 
 		field = zend_hash_find_ptr(&type->record.fields, field_name);
@@ -1334,10 +1332,8 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
 			type = ZEND_FFI_TYPE(type->pointer.type);
 		}
 		if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
-			if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
-				zend_throw_error(zend_ffi_exception_ce, "Attempt to assign field '%s' of non C struct/union", ZSTR_VAL(field_name));
-				return value;
-			}
+			zend_throw_error(zend_ffi_exception_ce, "Attempt to assign field '%s' of non C struct/union", ZSTR_VAL(field_name));
+			return value;
 		}
 
 		field = zend_hash_find_ptr(&type->record.fields, field_name);
@@ -3069,17 +3065,7 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
 		FFI_G(default_type_attr) = ZEND_FFI_ATTR_STORED;
 
 		if (zend_ffi_parse_decl(ZSTR_VAL(code), ZSTR_LEN(code)) == FAILURE) {
-			if (FFI_G(symbols)) {
-				zend_hash_destroy(FFI_G(symbols));
-				efree(FFI_G(symbols));
-				FFI_G(symbols) = NULL;
-			}
-			if (FFI_G(tags)) {
-				zend_hash_destroy(FFI_G(tags));
-				efree(FFI_G(tags));
-				FFI_G(tags) = NULL;
-			}
-			RETURN_THROWS();
+			goto cleanup;
 		}
 
 		if (FFI_G(symbols)) {
@@ -3091,7 +3077,7 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
 					addr = DL_FETCH_SYMBOL(handle, ZSTR_VAL(name));
 					if (!addr) {
 						zend_throw_error(zend_ffi_exception_ce, "Failed resolving C variable '%s'", ZSTR_VAL(name));
-						RETURN_THROWS();
+						goto cleanup;
 					}
 					sym->addr = addr;
 				} else if (sym->kind == ZEND_FFI_SYM_FUNC) {
@@ -3101,7 +3087,7 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
 					zend_string_release(mangled_name);
 					if (!addr) {
 						zend_throw_error(zend_ffi_exception_ce, "Failed resolving C function '%s'", ZSTR_VAL(name));
-						RETURN_THROWS();
+						goto cleanup;
 					}
 					sym->addr = addr;
 				}
@@ -3118,6 +3104,22 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
 	FFI_G(tags) = NULL;
 
 	RETURN_OBJ(&ffi->std);
+
+cleanup:
+	if (lib && handle) {
+		DL_UNLOAD(handle);
+	}
+	if (FFI_G(symbols)) {
+		zend_hash_destroy(FFI_G(symbols));
+		efree(FFI_G(symbols));
+		FFI_G(symbols) = NULL;
+	}
+	if (FFI_G(tags)) {
+		zend_hash_destroy(FFI_G(tags));
+		efree(FFI_G(tags));
+		FFI_G(tags) = NULL;
+	}
+	RETURN_THROWS();
 }
 /* }}} */
 
diff --git a/ext/ffi/tests/gh14286_2.phpt b/ext/ffi/tests/gh14286_2.phpt
index c3fc0b242a4..4dc48ccc27a 100644
--- a/ext/ffi/tests/gh14286_2.phpt
+++ b/ext/ffi/tests/gh14286_2.phpt
@@ -2,10 +2,6 @@
 GH-14286 (ffi enum type (when enum has no name) make memory leak)
 --EXTENSIONS--
 ffi
---SKIPIF--
-<?php
-if (PHP_DEBUG || getenv('SKIP_ASAN')) die("xfail: FFI cleanup after parser error is not implemented");
-?>
 --INI--
 ffi.enable=1
 --FILE--
diff --git a/ext/ffi/tests/gh18629_cdef_resolve_func.phpt b/ext/ffi/tests/gh18629_cdef_resolve_func.phpt
new file mode 100644
index 00000000000..ae81412efa6
--- /dev/null
+++ b/ext/ffi/tests/gh18629_cdef_resolve_func.phpt
@@ -0,0 +1,16 @@
+--TEST--
+GH-18629 (FFI::cdef() leaks on function resolution failure)
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+try {
+	$ffi = FFI::cdef("void nonexistent_ffi_test_func(void);");
+} catch (\FFI\Exception $e) {
+	echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Failed resolving C function 'nonexistent_ffi_test_func'
diff --git a/ext/ffi/tests/gh18629_read_field_non_struct.phpt b/ext/ffi/tests/gh18629_read_field_non_struct.phpt
new file mode 100644
index 00000000000..307fde1a53b
--- /dev/null
+++ b/ext/ffi/tests/gh18629_read_field_non_struct.phpt
@@ -0,0 +1,17 @@
+--TEST--
+GH-18629 (Read field of non C struct/union)
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+$x = FFI::cdef()->new("int*");
+try {
+	$y = $x->foo;
+} catch (\FFI\Exception $e) {
+	echo $e->getMessage() . "\n";
+}
+?>
+--EXPECT--
+Attempt to read field 'foo' of non C struct/union