From 7f5bf833a0520072f3b47c905e100463e4f3420c Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 31 Oct 2026 10:58:36 -0400 Subject: [PATCH] Fix use-after-free between MIME parts with mismatched lifetimes A part keeps raw pointers to its parent (part->parent) and to its last child (parsedata.lastpart). Since bd4a675 dropped the detaching child destructor, freeing a root while userland still holds a child leaves child->parent dangling, and MimeMessage::get_parent(), further mailparse_msg_parse() calls on the child, and position updates read or write the freed parent. Freeing in the other order is also unsafe: mailparse_msg_free() on a child force-closes it, leaving the parent's lastpart dangling and a closed resource in its children table, so continued parsing hits freed memory and mailparse_msg_get_structure() dereferences NULL. MimeMessage::remove() leaves the same dangling lastpart once the removed part is freed. php_mimepart_free() now clears the parent pointer of every surviving child, php_mimepart_remove_from_parent() clears the parent's lastpart, and mailparse_msg_free() returns true without freeing a part that still has a parent; that part is freed with its root. A child that outlives its parent is reported as a top-level part, as it was before bd4a675. Reported by Firas Fatnassi and Oz Kaiser. The child-detach change and the two outlives-parent tests are based on Oz Kaiser's patch. --- CHANGELOG.md | 1 + mailparse.c | 13 ++++++++- php_mailparse_mime.c | 13 +++++++-- tests/child_outlives_parent.phpt | 27 +++++++++++++++++++ tests/free_child_part.phpt | 33 +++++++++++++++++++++++ tests/mimemessage_remove_lastpart.phpt | 26 ++++++++++++++++++ tests/resource_child_outlives_parent.phpt | 28 +++++++++++++++++++ 7 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 tests/child_outlives_parent.phpt create mode 100644 tests/free_child_part.phpt create mode 100644 tests/mimemessage_remove_lastpart.phpt create mode 100644 tests/resource_child_outlives_parent.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f5e514..7d97044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Reject non-mimepart resources in `mailparse_msg_*` instead of crashing - Fix memory leaks - Fix `MimeMessage::extract_uue()` ignoring index greater than zero +- Fix use-after-free when a MIME part outlives its parent, or when a child part is freed before its parent. `mailparse_msg_free()` on a part that still belongs to a parent now returns true without freeing it; the part is freed with its root. (Firas Fatnassi, Oz Kaiser) # Version 3.2.0 - 2026-04-09 diff --git a/mailparse.c b/mailparse.c index 055fc6f..27c11dd 100644 --- a/mailparse.c +++ b/mailparse.c @@ -1155,10 +1155,21 @@ PHP_FUNCTION(mailparse_msg_parse_file) PHP_FUNCTION(mailparse_msg_free) { zval *arg; + zend_resource *res; + php_mimepart *part; + if (zend_parse_parameters(ZEND_NUM_ARGS(), "r", &arg) == FAILURE) { RETURN_FALSE; } - zend_list_close(Z_RES_P(arg)); + res = Z_RES_P(arg); + if (res->type == le_mime_part) { + part = res->ptr; + /* A child part is owned by its parent and freed along with it */ + if (part != NULL && part->parent != NULL) { + RETURN_TRUE; + } + } + zend_list_close(res); RETURN_TRUE; } /* }}} */ diff --git a/php_mailparse_mime.c b/php_mailparse_mime.c index 5e8e5bf..b8a5e5a 100644 --- a/php_mailparse_mime.c +++ b/php_mailparse_mime.c @@ -325,6 +325,7 @@ PHP_MAILPARSE_API php_mimepart *php_mimepart_alloc() PHP_MAILPARSE_API void php_mimepart_free(php_mimepart *part) { zval *childpart_z; + php_mimepart *childpart; HashPosition pos; /* Prevent the resource destructor from freeing this part again */ @@ -335,10 +336,15 @@ PHP_MAILPARSE_API void php_mimepart_free(php_mimepart *part) /* Release child resources via zval_ptr_dtor, which respects refcounts: * children with no external references are freed immediately, while * children held by userland (e.g. via mailparse_msg_get_part) stay - * alive until their own refcount reaches zero. The idempotent - * mimepart_dtor (res->ptr NULL guard) prevents double-free at shutdown. */ + * alive until their own refcount reaches zero, so they are detached + * from this part first. The idempotent mimepart_dtor (res->ptr NULL + * guard) prevents double-free at shutdown. */ zend_hash_internal_pointer_reset_ex(&part->children, &pos); while ((childpart_z = zend_hash_get_current_data_ex(&part->children, &pos)) != NULL) { + childpart = Z_RES_P(childpart_z)->ptr; + if (childpart != NULL) { + childpart->parent = NULL; + } zval_ptr_dtor(childpart_z); zend_hash_move_forward_ex(&part->children, &pos); } @@ -1010,6 +1016,9 @@ PHP_MAILPARSE_API void php_mimepart_remove_from_parent(php_mimepart *part) } part->parent = NULL; + if (parent->parsedata.lastpart == part) { + parent->parsedata.lastpart = NULL; + } zend_hash_internal_pointer_reset_ex(&parent->children, &pos); while((childpart_z = zend_hash_get_current_data_ex(&parent->children, &pos)) != NULL) { diff --git a/tests/child_outlives_parent.phpt b/tests/child_outlives_parent.phpt new file mode 100644 index 0000000..2ad64c0 --- /dev/null +++ b/tests/child_outlives_parent.phpt @@ -0,0 +1,27 @@ +--TEST-- +A child MimeMessage can safely outlive its parent +--SKIPIF-- + +--FILE-- +get_child(0); +} + +$data = "MIME-Version: 1.0\r\n" + . "Content-Type: multipart/mixed; boundary=x\r\n\r\n" + . "--x\r\nContent-Type: text/plain\r\n\r\nbody\r\n--x--\r\n"; + +$child = firstChild($data); +var_dump($child instanceof MimeMessage); +var_dump($child->get_parent()); +var_dump($child->get_child_count()); +?> +--EXPECT-- +bool(true) +NULL +int(0) diff --git a/tests/free_child_part.phpt b/tests/free_child_part.phpt new file mode 100644 index 0000000..5dd5543 --- /dev/null +++ b/tests/free_child_part.phpt @@ -0,0 +1,33 @@ +--TEST-- +mailparse_msg_free() on a child part leaves the parent intact +--SKIPIF-- + +--FILE-- +\r\n--x--\r\n"); +var_dump(mailparse_msg_get_structure($root)); +var_dump(mailparse_msg_get_part_data(mailparse_msg_get_part($root, '1.1'))['content-type']); +var_dump(mailparse_msg_free($root)); +?> +--EXPECT-- +bool(true) +array(3) { + [0]=> + string(1) "1" + [1]=> + string(3) "1.1" + [2]=> + string(3) "1.2" +} +string(10) "text/plain" +bool(true) diff --git a/tests/mimemessage_remove_lastpart.phpt b/tests/mimemessage_remove_lastpart.phpt new file mode 100644 index 0000000..c150c1d --- /dev/null +++ b/tests/mimemessage_remove_lastpart.phpt @@ -0,0 +1,26 @@ +--TEST-- +Freeing a part removed with MimeMessage::remove() leaves its former parent intact +--SKIPIF-- + +--FILE-- +get_child(1); +$child->remove(); + +var_dump(mailparse_msg_free(((array)$child)[0])); +$res = ((array)$root)[0]; +var_dump(mailparse_msg_parse($res, "more\r\n")); +var_dump(mailparse_msg_get_structure($res)); +?> +--EXPECT-- +bool(true) +bool(true) +array(1) { + [0]=> + string(1) "1" +} diff --git a/tests/resource_child_outlives_parent.phpt b/tests/resource_child_outlives_parent.phpt new file mode 100644 index 0000000..3ca2987 --- /dev/null +++ b/tests/resource_child_outlives_parent.phpt @@ -0,0 +1,28 @@ +--TEST-- +A child mailparse resource can safely outlive its parent +--SKIPIF-- + +--FILE-- + +--EXPECT-- +bool(true) +bool(true) +string(10) "text/plain" +int(4) +bool(true)