Skip to content

Commit 19c1e20

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
1 parent 71ddede commit 19c1e20

File tree

3 files changed

+84
-3
lines changed

3 files changed

+84
-3
lines changed
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 (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
8+
?>
9+
--INI--
10+
memory_limit="5G"
11+
--FILE--
12+
<?php
13+
14+
$path = __DIR__ . '/too_large_test.html';
15+
$file = fopen($path, 'w+');
16+
17+
// Write over 4GB
18+
const MIN_FILE_SIZE = 4_294_967_295;
19+
$total_bytes = 0;
20+
$s = str_repeat("a", 10_000);
21+
while ($total_bytes < MIN_FILE_SIZE) {
22+
$bytes_written = fwrite($file, $s);
23+
if ($bytes_written === false) {
24+
echo "Didn't write bytes\n";
25+
}
26+
$total_bytes += $bytes_written;
27+
}
28+
29+
$tidy = new tidy;
30+
try {
31+
var_dump($tidy->parseFile($path));
32+
} catch (\Throwable $e) {
33+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
34+
}
35+
36+
try {
37+
var_dump(tidy_parse_file($path));
38+
} catch (\Throwable $e) {
39+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
40+
}
41+
42+
try {
43+
$tidy = new tidy($path);
44+
} catch (\Throwable $e) {
45+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
46+
}
47+
?>
48+
--CLEAN--
49+
<?php
50+
$path = __DIR__ . '/too_large_test.html';
51+
unlink($path);
52+
?>
53+
--EXPECT--
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)