From 793f22f4296e7f8b0a40157df7b6ce26aadbbd0c Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Sat, 29 Jul 2023 17:03:20 +0200 Subject: [PATCH] Fix uaf of MBSTRG(all_encodings_list) We need to remove the value from the GC buffer before freeing it. Otherwise shutdown will uaf when running the gc. Do that by switching from zend_hash_destroy to zend_array_destroy, which should also be faster for freeing members due to inlining of i_zval_ptr_dtor. --- ext/mbstring/mbstring.c | 3 +-- ext/mbstring/tests/mb_list_encodings_gc_uaf.phpt | 9 +++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 ext/mbstring/tests/mb_list_encodings_gc_uaf.phpt diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index 13ad7952b9864..3e59806b86750 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -1159,8 +1159,7 @@ PHP_RSHUTDOWN_FUNCTION(mbstring) if (MBSTRG(all_encodings_list)) { GC_DELREF(MBSTRG(all_encodings_list)); - zend_hash_destroy(MBSTRG(all_encodings_list)); - efree(MBSTRG(all_encodings_list)); + zend_array_destroy(MBSTRG(all_encodings_list)); MBSTRG(all_encodings_list) = NULL; } diff --git a/ext/mbstring/tests/mb_list_encodings_gc_uaf.phpt b/ext/mbstring/tests/mb_list_encodings_gc_uaf.phpt new file mode 100644 index 0000000000000..4a83372b16744 --- /dev/null +++ b/ext/mbstring/tests/mb_list_encodings_gc_uaf.phpt @@ -0,0 +1,9 @@ +--TEST-- +Use-after-free of MBSTRG(all_encodings_list) on shutdown +--EXTENSIONS-- +mbstring +--FILE-- + +--EXPECT--