From 18183ff9c745290d963dfdae1bf55d2e4b912f17 Mon Sep 17 00:00:00 2001
From: Dmitry Stogov <dmitry@zend.com>
Date: Thu, 11 Aug 2022 09:30:47 +0300
Subject: [PATCH] Fix order of checks to throw exception with better message

This clarifies the "->cdata" meaning.
---
 ext/ffi/ffi.c          | 59 +++++++++++++++++++++++-------------------
 ext/ffi/tests/047.phpt | 38 +++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 26 deletions(-)
 create mode 100644 ext/ffi/tests/047.phpt

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 8f05686367a..686db459eff 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -1169,20 +1169,10 @@ static zval *zend_ffi_cdata_read_field(zend_object *obj, zend_string *field_name
 	if (cache_slot && *cache_slot == type) {
 		field = *(cache_slot + 1);
 	} else {
+		if (type->kind == ZEND_FFI_TYPE_POINTER) {
+			type = ZEND_FFI_TYPE(type->pointer.type);
+		}
 		if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
-			if (type->kind == ZEND_FFI_TYPE_POINTER) {
-				/* transparently dereference the pointer */
-				if (UNEXPECTED(!ptr)) {
-					zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
-					return &EG(uninitialized_zval);
-				}
-				ptr = (void*)(*(char**)ptr);
-				if (UNEXPECTED(!ptr)) {
-					zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
-					return &EG(uninitialized_zval);
-				}
-				type = ZEND_FFI_TYPE(type->pointer.type);
-			}
 			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);
@@ -1201,6 +1191,20 @@ static zval *zend_ffi_cdata_read_field(zend_object *obj, zend_string *field_name
 		}
 	}
 
+	if (ZEND_FFI_TYPE(cdata->type)->kind == ZEND_FFI_TYPE_POINTER) {
+		/* transparently dereference the pointer */
+		if (UNEXPECTED(!ptr)) {
+			zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
+			return &EG(uninitialized_zval);
+		}
+		ptr = (void*)(*(char**)ptr);
+		if (UNEXPECTED(!ptr)) {
+			zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
+			return &EG(uninitialized_zval);
+		}
+		type = ZEND_FFI_TYPE(type->pointer.type);
+	}
+
 #if 0
 	if (UNEXPECTED(!ptr)) {
 		zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
@@ -1238,20 +1242,10 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
 	if (cache_slot && *cache_slot == type) {
 		field = *(cache_slot + 1);
 	} else {
+		if (type->kind == ZEND_FFI_TYPE_POINTER) {
+			type = ZEND_FFI_TYPE(type->pointer.type);
+		}
 		if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
-			if (type->kind == ZEND_FFI_TYPE_POINTER) {
-				/* transparently dereference the pointer */
-				if (UNEXPECTED(!ptr)) {
-					zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
-					return value;
-				}
-				ptr = (void*)(*(char**)ptr);
-				if (UNEXPECTED(!ptr)) {
-					zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
-					return value;
-				}
-				type = ZEND_FFI_TYPE(type->pointer.type);
-			}
 			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;
@@ -1270,6 +1264,19 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
 		}
 	}
 
+	if (ZEND_FFI_TYPE(cdata->type)->kind == ZEND_FFI_TYPE_POINTER) {
+		/* transparently dereference the pointer */
+		if (UNEXPECTED(!ptr)) {
+			zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
+			return value;
+		}
+		ptr = (void*)(*(char**)ptr);
+		if (UNEXPECTED(!ptr)) {
+			zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
+			return value;
+		}
+	}
+
 #if 0
 	if (UNEXPECTED(!ptr)) {
 		zend_throw_error(zend_ffi_exception_ce, "NULL pointer dereference");
diff --git a/ext/ffi/tests/047.phpt b/ext/ffi/tests/047.phpt
new file mode 100644
index 00000000000..8bd83320b50
--- /dev/null
+++ b/ext/ffi/tests/047.phpt
@@ -0,0 +1,38 @@
+--TEST--
+FFI 047: FFI::CData->cdata meaning
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+$x = FFI::new("int");
+$x->cdata = 42;
+var_dump($x);
+
+$x = FFI::new("int*");
+try {
+	$x->cdata = 42;
+    var_dump($x);
+} catch (Throwable $e) {
+	echo $e->getMessage() . "\n";
+}
+
+$x = FFI::new("struct {int cdata;}");
+try {
+	$x->cdata = 42;
+    var_dump($x);
+} catch (Throwable $e) {
+	echo $e->getMessage() . "\n";
+}
+?>
+--EXPECTF--
+object(FFI\CData:int32_t)#%d (1) {
+  ["cdata"]=>
+  int(42)
+}
+Attempt to assign field 'cdata' of non C struct/union
+object(FFI\CData:struct <anonymous>)#%d (1) {
+  ["cdata"]=>
+  int(42)
+}