Skip to content

Commit acd5fe8

Browse files
iliaalLamentXU123
iliaal
authored and
LamentXU123
committed
ext/zip: Reject ZipArchive mutators during close() (#24025)
A progress or cancel callback runs inside zip_close() after libzip has fixed the list of entries it writes. Deleting or unchanging an entry from the callback frees a dirent that zip_close() still uses. Other changes are either silently dropped or make close() fail and lose the archive. Reject archive mutations with the same "Already being closed" Error that close() and open() already raise, using php_zipobj_closing(). This follows GH-23749 and complements the destructor protection in GH-23779. Cover progress callbacks, cancel callbacks, and the implicit close() in open(), and verify that the archive contents are preserved. Closes #24025
1 parent ddbb581 commit acd5fe8

5 files changed

Lines changed: 397 additions & 4 deletions

File tree

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ PHP NEWS
3131
- Zip:
3232
. Fixed use-after-free when re-entering ZipArchive during destruction or
3333
a close warning, and rejected opening streams while closing. (jvoisin)
34+
. Fixed a use-after-free when a ZipArchive method that modifies the archive
35+
is called from a progress or cancel callback during close().
36+
(Ilia Alshanetsky)
3437

3538
22 Oct 2026, PHP 8.4.27
3639

‎ext/zip/php_zip.c‎

Lines changed: 123 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,15 @@ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */
638638
}
639639
/* }}} */
640640

641+
static bool php_zipobj_closing(ze_zip_object *obj)
642+
{
643+
if (obj->archive && obj->archive->close) {
644+
zend_throw_error(NULL, "Already being closed");
645+
return true;
646+
}
647+
return false;
648+
}
649+
641650
#ifdef HAVE_GLOB /* {{{ */
642651
#ifndef GLOB_ONLYDIR
643652
#define GLOB_ONLYDIR (1<<30)
@@ -1596,9 +1605,8 @@ PHP_METHOD(ZipArchive, open)
15961605

15971606
if (ze_obj->archive) {
15981607
/* we already have an opened zip, free it */
1599-
if (ze_obj->archive->close) {
1608+
if (php_zipobj_closing(ze_obj)) {
16001609
efree(resolved_path);
1601-
zend_throw_error(NULL, "Already being closed");
16021610
RETURN_THROWS();
16031611
}
16041612
intern = ze_obj->archive->za;
@@ -1661,6 +1669,10 @@ PHP_METHOD(ZipArchive, setPassword)
16611669

16621670
ZIP_FROM_OBJECT(intern, self);
16631671

1672+
if (php_zipobj_closing(Z_ZIP_P(self))) {
1673+
RETURN_THROWS();
1674+
}
1675+
16641676
if (password_len < 1) {
16651677
RETURN_FALSE;
16661678
}
@@ -1690,8 +1702,7 @@ PHP_METHOD(ZipArchive, close)
16901702

16911703
ze_obj = Z_ZIP_P(self);
16921704

1693-
if (ze_obj->archive->close) {
1694-
zend_throw_error(NULL, "Already being closed");
1705+
if (php_zipobj_closing(ze_obj)) {
16951706
RETURN_THROWS();
16961707
}
16971708

@@ -1836,6 +1847,10 @@ PHP_METHOD(ZipArchive, addEmptyDir)
18361847

18371848
ZIP_FROM_OBJECT(intern, self);
18381849

1850+
if (php_zipobj_closing(Z_ZIP_P(self))) {
1851+
RETURN_THROWS();
1852+
}
1853+
18391854
if (dirname_len<1) {
18401855
RETURN_FALSE;
18411856
}
@@ -1894,6 +1909,10 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /*
18941909
RETURN_THROWS();
18951910
}
18961911

1912+
if (php_zipobj_closing(Z_ZIP_P(self))) {
1913+
RETURN_THROWS();
1914+
}
1915+
18971916
if (type == 1) {
18981917
found = php_zip_glob(ZSTR_VAL(pattern), ZSTR_LEN(pattern), glob_flags, return_value);
18991918
} else {
@@ -2028,6 +2047,10 @@ PHP_METHOD(ZipArchive, addFile)
20282047
entry_name_len = ZSTR_LEN(filename);
20292048
}
20302049

2050+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2051+
RETURN_THROWS();
2052+
}
2053+
20312054
if (php_zip_add_file(Z_ZIP_P(self), ZSTR_VAL(filename), ZSTR_LEN(filename),
20322055
entry_name, entry_name_len, offset_start, offset_len, -1, flags) < 0) {
20332056
RETURN_FALSE;
@@ -2061,6 +2084,10 @@ PHP_METHOD(ZipArchive, replaceFile)
20612084
RETURN_THROWS();
20622085
}
20632086

2087+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2088+
RETURN_THROWS();
2089+
}
2090+
20642091
if (php_zip_add_file(Z_ZIP_P(self), ZSTR_VAL(filename), ZSTR_LEN(filename),
20652092
NULL, 0, offset_start, offset_len, index, flags) < 0) {
20662093
RETURN_FALSE;
@@ -2091,6 +2118,10 @@ PHP_METHOD(ZipArchive, addFromString)
20912118

20922119
ZIP_FROM_OBJECT(intern, self);
20932120

2121+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2122+
RETURN_THROWS();
2123+
}
2124+
20942125
ze_obj = Z_ZIP_P(self);
20952126
archive = ze_obj->archive;
20962127
if (archive->buffers_cnt) {
@@ -2233,6 +2264,10 @@ PHP_METHOD(ZipArchive, setArchiveComment)
22332264

22342265
ZIP_FROM_OBJECT(intern, self);
22352266

2267+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2268+
RETURN_THROWS();
2269+
}
2270+
22362271
if (comment_len > 0xffff) {
22372272
zend_argument_value_error(1, "must be less than 65535 bytes");
22382273
RETURN_THROWS();
@@ -2281,6 +2316,10 @@ PHP_METHOD(ZipArchive, setArchiveFlag)
22812316

22822317
ZIP_FROM_OBJECT(intern, self);
22832318

2319+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2320+
RETURN_THROWS();
2321+
}
2322+
22842323
if (zip_set_archive_flag(intern, flag, (int)value)) {
22852324
RETURN_FALSE;
22862325
} else {
@@ -2324,6 +2363,10 @@ PHP_METHOD(ZipArchive, setCommentName)
23242363

23252364
ZIP_FROM_OBJECT(intern, self);
23262365

2366+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2367+
RETURN_THROWS();
2368+
}
2369+
23272370
if (comment_len > 0xffff) {
23282371
zend_argument_value_error(2, "must be less than 65535 bytes");
23292372
RETURN_THROWS();
@@ -2355,6 +2398,10 @@ PHP_METHOD(ZipArchive, setCommentIndex)
23552398

23562399
ZIP_FROM_OBJECT(intern, self);
23572400

2401+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2402+
RETURN_THROWS();
2403+
}
2404+
23582405
if (comment_len > 0xffff) {
23592406
zend_argument_value_error(2, "must be less than 65535 bytes");
23602407
RETURN_THROWS();
@@ -2387,6 +2434,10 @@ PHP_METHOD(ZipArchive, setExternalAttributesName)
23872434

23882435
ZIP_FROM_OBJECT(intern, self);
23892436

2437+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2438+
RETURN_THROWS();
2439+
}
2440+
23902441
if (name_len == 0) {
23912442
zend_argument_must_not_be_empty_error(1);
23922443
RETURN_THROWS();
@@ -2420,6 +2471,10 @@ PHP_METHOD(ZipArchive, setExternalAttributesIndex)
24202471

24212472
ZIP_FROM_OBJECT(intern, self);
24222473

2474+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2475+
RETURN_THROWS();
2476+
}
2477+
24232478
PHP_ZIP_STAT_INDEX(intern, index, 0, sb);
24242479
if (zip_file_set_external_attributes(intern, (zip_uint64_t)index,
24252480
(zip_flags_t)flags, (zip_uint8_t)(opsys&0xff), (zip_uint32_t)attr) < 0) {
@@ -2515,6 +2570,10 @@ PHP_METHOD(ZipArchive, setEncryptionName)
25152570

25162571
ZIP_FROM_OBJECT(intern, self);
25172572

2573+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2574+
RETURN_THROWS();
2575+
}
2576+
25182577
if (name_len == 0) {
25192578
zend_argument_must_not_be_empty_error(1);
25202579
RETURN_THROWS();
@@ -2554,6 +2613,10 @@ PHP_METHOD(ZipArchive, setEncryptionIndex)
25542613

25552614
ZIP_FROM_OBJECT(intern, self);
25562615

2616+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2617+
RETURN_THROWS();
2618+
}
2619+
25572620
if (UNEXPECTED(zip_file_set_encryption(intern, index, ZIP_EM_NONE, NULL) < 0)) {
25582621
php_error_docref(NULL, E_WARNING, "password reset failed");
25592622
RETURN_FALSE;
@@ -2642,6 +2705,10 @@ PHP_METHOD(ZipArchive, setCompressionName)
26422705

26432706
ZIP_FROM_OBJECT(intern, this);
26442707

2708+
if (php_zipobj_closing(Z_ZIP_P(this))) {
2709+
RETURN_THROWS();
2710+
}
2711+
26452712
if (name_len == 0) {
26462713
zend_argument_must_not_be_empty_error(1);
26472714
RETURN_THROWS();
@@ -2676,6 +2743,10 @@ PHP_METHOD(ZipArchive, setCompressionIndex)
26762743

26772744
ZIP_FROM_OBJECT(intern, this);
26782745

2746+
if (php_zipobj_closing(Z_ZIP_P(this))) {
2747+
RETURN_THROWS();
2748+
}
2749+
26792750
if (zip_set_file_compression(intern, (zip_uint64_t)index,
26802751
(zip_int32_t)comp_method, (zip_uint32_t)comp_flags) != 0) {
26812752
RETURN_FALSE;
@@ -2702,6 +2773,10 @@ PHP_METHOD(ZipArchive, setMtimeName)
27022773

27032774
ZIP_FROM_OBJECT(intern, this);
27042775

2776+
if (php_zipobj_closing(Z_ZIP_P(this))) {
2777+
RETURN_THROWS();
2778+
}
2779+
27052780
if (name_len == 0) {
27062781
zend_argument_must_not_be_empty_error(1);
27072782
RETURN_THROWS();
@@ -2736,6 +2811,10 @@ PHP_METHOD(ZipArchive, setMtimeIndex)
27362811

27372812
ZIP_FROM_OBJECT(intern, this);
27382813

2814+
if (php_zipobj_closing(Z_ZIP_P(this))) {
2815+
RETURN_THROWS();
2816+
}
2817+
27392818
if (zip_file_set_mtime(intern, (zip_uint64_t)index,
27402819
(time_t)mtime, (zip_uint32_t)flags) != 0) {
27412820
RETURN_FALSE;
@@ -2758,6 +2837,10 @@ PHP_METHOD(ZipArchive, deleteIndex)
27582837

27592838
ZIP_FROM_OBJECT(intern, self);
27602839

2840+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2841+
RETURN_THROWS();
2842+
}
2843+
27612844
if (index < 0) {
27622845
RETURN_FALSE;
27632846
}
@@ -2785,6 +2868,10 @@ PHP_METHOD(ZipArchive, deleteName)
27852868

27862869
ZIP_FROM_OBJECT(intern, self);
27872870

2871+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2872+
RETURN_THROWS();
2873+
}
2874+
27882875
if (name_len < 1) {
27892876
RETURN_FALSE;
27902877
}
@@ -2816,6 +2903,10 @@ PHP_METHOD(ZipArchive, renameIndex)
28162903

28172904
ZIP_FROM_OBJECT(intern, self);
28182905

2906+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2907+
RETURN_THROWS();
2908+
}
2909+
28192910
if (new_name_len == 0) {
28202911
zend_argument_must_not_be_empty_error(2);
28212912
RETURN_THROWS();
@@ -2844,6 +2935,10 @@ PHP_METHOD(ZipArchive, renameName)
28442935

28452936
ZIP_FROM_OBJECT(intern, self);
28462937

2938+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2939+
RETURN_THROWS();
2940+
}
2941+
28472942
if (new_name_len == 0) {
28482943
zend_argument_must_not_be_empty_error(2);
28492944
RETURN_THROWS();
@@ -2872,6 +2967,10 @@ PHP_METHOD(ZipArchive, unchangeIndex)
28722967

28732968
ZIP_FROM_OBJECT(intern, self);
28742969

2970+
if (php_zipobj_closing(Z_ZIP_P(self))) {
2971+
RETURN_THROWS();
2972+
}
2973+
28752974
if (index < 0) {
28762975
RETURN_FALSE;
28772976
}
@@ -2899,6 +2998,10 @@ PHP_METHOD(ZipArchive, unchangeName)
28992998

29002999
ZIP_FROM_OBJECT(intern, self);
29013000

3001+
if (php_zipobj_closing(Z_ZIP_P(self))) {
3002+
RETURN_THROWS();
3003+
}
3004+
29023005
if (name_len < 1) {
29033006
RETURN_FALSE;
29043007
}
@@ -2925,6 +3028,10 @@ PHP_METHOD(ZipArchive, unchangeAll)
29253028

29263029
ZIP_FROM_OBJECT(intern, self);
29273030

3031+
if (php_zipobj_closing(Z_ZIP_P(self))) {
3032+
RETURN_THROWS();
3033+
}
3034+
29283035
if (zip_unchange_all(intern) != 0) {
29293036
RETURN_FALSE;
29303037
} else {
@@ -2945,6 +3052,10 @@ PHP_METHOD(ZipArchive, unchangeArchive)
29453052

29463053
ZIP_FROM_OBJECT(intern, self);
29473054

3055+
if (php_zipobj_closing(Z_ZIP_P(self))) {
3056+
RETURN_THROWS();
3057+
}
3058+
29483059
if (zip_unchange_archive(intern) != 0) {
29493060
RETURN_FALSE;
29503061
} else {
@@ -3240,6 +3351,10 @@ PHP_METHOD(ZipArchive, registerProgressCallback)
32403351

32413352
ZIP_FROM_OBJECT(intern, self);
32423353

3354+
if (php_zipobj_closing(Z_ZIP_P(self))) {
3355+
RETURN_THROWS();
3356+
}
3357+
32433358
archive = Z_ZIP_P(self)->archive;
32443359

32453360
/* register */
@@ -3282,6 +3397,10 @@ PHP_METHOD(ZipArchive, registerCancelCallback)
32823397

32833398
ZIP_FROM_OBJECT(intern, self);
32843399

3400+
if (php_zipobj_closing(Z_ZIP_P(self))) {
3401+
RETURN_THROWS();
3402+
}
3403+
32853404
archive = Z_ZIP_P(self)->archive;
32863405

32873406
/* register */

0 commit comments

Comments
 (0)