Skip to content

Commit 704aadd

Browse files
committed
Fix memory leaks in ext-tidy
We must not instantiate the object prior checking error conditions Moreover, we need to release the HUGE amount of memory for files which are over 4GB when throwing a ValueError Closes GH-10545
1 parent 8c8a38a commit 704aadd

File tree

4 files changed

+88
-3
lines changed

4 files changed

+88
-3
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ PHP NEWS
4646
. Fix bug GH-9697 for reset/end/next/prev() attempting to move pointer of
4747
properties table for certain internal classes such as FFI classes
4848

49+
- Tidy:
50+
. Fix memory leaks when attempting to open a non-existing file or a file over
51+
4GB. (Girgias)
52+
4953
02 Feb 2023, PHP 8.1.15
5054

5155
- Apache:
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
Trying to parse a file that is too large (over 4GB)
3+
--EXTENSIONS--
4+
tidy
5+
--SKIPIF--
6+
<?php
7+
if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only");
8+
if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
9+
?>
10+
--INI--
11+
memory_limit="5G"
12+
--FILE--
13+
<?php
14+
15+
$path = __DIR__ . '/too_large_test.html';
16+
$file = fopen($path, 'w+');
17+
18+
// Write over 4GB
19+
const MIN_FILE_SIZE = 4_294_967_295;
20+
21+
var_dump(fseek($file, MIN_FILE_SIZE+10));
22+
$s = str_repeat("a", 10);
23+
$bytes_written = fwrite($file, $s);
24+
if ($bytes_written === false) {
25+
echo "Didn't write bytes\n";
26+
}
27+
28+
$tidy = new tidy;
29+
try {
30+
var_dump($tidy->parseFile($path));
31+
} catch (\Throwable $e) {
32+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
33+
}
34+
35+
try {
36+
var_dump(tidy_parse_file($path));
37+
} catch (\Throwable $e) {
38+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
39+
}
40+
41+
try {
42+
$tidy = new tidy($path);
43+
} catch (\Throwable $e) {
44+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
45+
}
46+
?>
47+
--CLEAN--
48+
<?php
49+
$path = __DIR__ . '/too_large_test.html';
50+
unlink($path);
51+
?>
52+
--EXPECT--
53+
int(0)
54+
ValueError: Input string is too long
55+
ValueError: Input string is too long
56+
ValueError: Input string is too long
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
Trying to parse a non existent file
3+
--EXTENSIONS--
4+
tidy
5+
--FILE--
6+
<?php
7+
8+
$tidy = new tidy;
9+
var_dump($tidy->parseFile("does_not_exist.html"));
10+
11+
var_dump(tidy_parse_file("does_not_exist.html"));
12+
13+
$tidy = new tidy("does_not_exist.html");
14+
?>
15+
--EXPECTF--
16+
Warning: tidy::parseFile(): Cannot load "does_not_exist.html" into memory in %s on line %d
17+
bool(false)
18+
19+
Warning: tidy_parse_file(): Cannot load "does_not_exist.html" into memory in %s on line %d
20+
bool(false)
21+
22+
Warning: tidy::__construct(): Cannot load "does_not_exist.html" into memory in %s on line %d

ext/tidy/tidy.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,19 +1059,20 @@ PHP_FUNCTION(tidy_parse_file)
10591059
Z_PARAM_BOOL(use_include_path)
10601060
ZEND_PARSE_PARAMETERS_END();
10611061

1062-
tidy_instanciate(tidy_ce_doc, return_value);
1063-
obj = Z_TIDY_P(return_value);
1064-
10651062
if (!(contents = php_tidy_file_to_mem(ZSTR_VAL(inputfile), use_include_path))) {
10661063
php_error_docref(NULL, E_WARNING, "Cannot load \"%s\" into memory%s", ZSTR_VAL(inputfile), (use_include_path) ? " (using include path)" : "");
10671064
RETURN_FALSE;
10681065
}
10691066

10701067
if (ZEND_SIZE_T_UINT_OVFL(ZSTR_LEN(contents))) {
1068+
zend_string_release_ex(contents, 0);
10711069
zend_value_error("Input string is too long");
10721070
RETURN_THROWS();
10731071
}
10741072

1073+
tidy_instanciate(tidy_ce_doc, return_value);
1074+
obj = Z_TIDY_P(return_value);
1075+
10751076
TIDY_APPLY_CONFIG(obj->ptdoc->doc, options_str, options_ht);
10761077

10771078
if (php_tidy_parse_string(obj, ZSTR_VAL(contents), (uint32_t)ZSTR_LEN(contents), enc) == FAILURE) {
@@ -1362,6 +1363,7 @@ PHP_METHOD(tidy, __construct)
13621363
}
13631364

13641365
if (ZEND_SIZE_T_UINT_OVFL(ZSTR_LEN(contents))) {
1366+
zend_string_release_ex(contents, 0);
13651367
zend_value_error("Input string is too long");
13661368
RETURN_THROWS();
13671369
}
@@ -1400,6 +1402,7 @@ PHP_METHOD(tidy, parseFile)
14001402
}
14011403

14021404
if (ZEND_SIZE_T_UINT_OVFL(ZSTR_LEN(contents))) {
1405+
zend_string_release_ex(contents, 0);
14031406
zend_value_error("Input string is too long");
14041407
RETURN_THROWS();
14051408
}

0 commit comments

Comments
 (0)