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)