Skip to content

Commit f3c58a5

Browse files
Dik Takkennikic
Dik Takken
authored and
nikic
committed
Make handling of NULL bytes in file paths more consistent (WIP)
Not all extensions consistently throw exceptions when the user passes a path name containing null bytes. Also, some extensions would throw a ValueError while others would throw a TypeError. Error messages also varied. Now a ValueError is thrown after all failed path checks, at least for as far as these occur in functions that are exposed to userland. Closes GH-6216.
1 parent 59aa21c commit f3c58a5

14 files changed

+137
-65
lines changed

‎ext/dom/document.c

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,6 +1197,7 @@ static xmlDocPtr dom_document_parser(zval *id, int mode, char *source, size_t so
11971197
if (mode == DOM_LOAD_FILE) {
11981198
char *file_dest;
11991199
if (CHECK_NULL_PATH(source, source_len)) {
1200+
zend_value_error("Path to document must not contain any null bytes");
12001201
return NULL;
12011202
}
12021203
file_dest = _dom_get_valid_file_path(source, resolved_path, MAXPATHLEN);
@@ -1305,8 +1306,8 @@ static void dom_parse_document(INTERNAL_FUNCTION_PARAMETERS, int mode) {
13051306
}
13061307

13071308
if (!source_len) {
1308-
php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
1309-
RETURN_FALSE;
1309+
zend_argument_value_error(1, "must not be empty");
1310+
RETURN_THROWS();
13101311
}
13111312
if (ZEND_SIZE_T_INT_OVFL(source_len)) {
13121313
php_error_docref(NULL, E_WARNING, "Input string is too long");
@@ -1388,8 +1389,8 @@ PHP_METHOD(DOMDocument, save)
13881389
}
13891390

13901391
if (file_len == 0) {
1391-
php_error_docref(NULL, E_WARNING, "Invalid Filename");
1392-
RETURN_FALSE;
1392+
zend_argument_value_error(1, "must not be empty");
1393+
RETURN_THROWS();
13931394
}
13941395

13951396
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
@@ -1623,18 +1624,18 @@ static void _dom_document_schema_validate(INTERNAL_FUNCTION_PARAMETERS, int type
16231624
RETURN_THROWS();
16241625
}
16251626

1626-
if (source_len == 0) {
1627-
php_error_docref(NULL, E_WARNING, "Invalid Schema source");
1628-
RETURN_FALSE;
1627+
if (!source_len) {
1628+
zend_argument_value_error(1, "must not be empty");
1629+
RETURN_THROWS();
16291630
}
16301631

16311632
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
16321633

16331634
switch (type) {
16341635
case DOM_LOAD_FILE:
16351636
if (CHECK_NULL_PATH(source, source_len)) {
1636-
php_error_docref(NULL, E_WARNING, "Invalid Schema file source");
1637-
RETURN_FALSE;
1637+
zend_argument_value_error(1, "must not contain any null bytes");
1638+
RETURN_THROWS();
16381639
}
16391640
valid_file = _dom_get_valid_file_path(source, resolved_path, MAXPATHLEN);
16401641
if (!valid_file) {
@@ -1724,18 +1725,18 @@ static void _dom_document_relaxNG_validate(INTERNAL_FUNCTION_PARAMETERS, int typ
17241725
RETURN_THROWS();
17251726
}
17261727

1727-
if (source_len == 0) {
1728-
php_error_docref(NULL, E_WARNING, "Invalid Schema source");
1729-
RETURN_FALSE;
1728+
if (!source_len) {
1729+
zend_argument_value_error(1, "must not be empty");
1730+
RETURN_THROWS();
17301731
}
17311732

17321733
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
17331734

17341735
switch (type) {
17351736
case DOM_LOAD_FILE:
17361737
if (CHECK_NULL_PATH(source, source_len)) {
1737-
php_error_docref(NULL, E_WARNING, "Invalid RelaxNG file source");
1738-
RETURN_FALSE;
1738+
zend_argument_value_error(1, "must not contain any null bytes");
1739+
RETURN_THROWS();
17391740
}
17401741
valid_file = _dom_get_valid_file_path(source, resolved_path, MAXPATHLEN);
17411742
if (!valid_file) {
@@ -1823,8 +1824,8 @@ static void dom_load_html(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ */
18231824
}
18241825

18251826
if (!source_len) {
1826-
php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
1827-
RETURN_FALSE;
1827+
zend_argument_value_error(1, "must not be empty");
1828+
RETURN_THROWS();
18281829
}
18291830

18301831
if (ZEND_LONG_EXCEEDS_INT(options)) {
@@ -1834,8 +1835,8 @@ static void dom_load_html(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ */
18341835

18351836
if (mode == DOM_LOAD_FILE) {
18361837
if (CHECK_NULL_PATH(source, source_len)) {
1837-
php_error_docref(NULL, E_WARNING, "Invalid file source");
1838-
RETURN_FALSE;
1838+
zend_argument_value_error(1, "must not contain any null bytes");
1839+
RETURN_THROWS();
18391840
}
18401841
ctxt = htmlCreateFileParserCtxt(source, NULL);
18411842
} else {
@@ -1930,8 +1931,8 @@ PHP_METHOD(DOMDocument, saveHTMLFile)
19301931
}
19311932

19321933
if (file_len == 0) {
1933-
php_error_docref(NULL, E_WARNING, "Invalid Filename");
1934-
RETURN_FALSE;
1934+
zend_argument_value_error(1, "must not be empty");
1935+
RETURN_THROWS();
19351936
}
19361937

19371938
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);

‎ext/dom/tests/DOMDocument_loadHTML_error2.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ require_once('skipif.inc');
99
--FILE--
1010
<?php
1111
$doc = new DOMDocument();
12-
$doc->loadHTML('');
12+
try {
13+
$doc->loadHTML('');
14+
} catch (ValueError $e) {
15+
echo $e->getMessage() . "\n";
16+
}
1317
?>
14-
--EXPECTF--
15-
Warning: DOMDocument::loadHTML(): Empty string supplied as input in %s on line %d
18+
--EXPECT--
19+
DOMDocument::loadHTML(): Argument #1 ($source) must not be empty

‎ext/dom/tests/DOMDocument_loadHTMLfile_error2.phpt

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,19 @@ assert.bail=true
1111
--FILE--
1212
<?php
1313
$doc = new DOMDocument();
14-
$result = $doc->loadHTMLFile("");
15-
assert($result === false);
14+
try {
15+
$result = $doc->loadHTMLFile("");
16+
} catch (ValueError $e) {
17+
echo $e->getMessage() . "\n";
18+
}
19+
1620
$doc = new DOMDocument();
17-
$result = $doc->loadHTMLFile("text.html\0something");
18-
assert($result === false);
21+
try {
22+
$result = $doc->loadHTMLFile("text.html\0something");
23+
} catch (ValueError $e) {
24+
echo $e->getMessage() . "\n";
25+
}
1926
?>
20-
--EXPECTF--
21-
%r(PHP ){0,1}%rWarning: DOMDocument::loadHTMLFile(): Empty string supplied as input %s
22-
23-
%r(PHP ){0,1}%rWarning: DOMDocument::loadHTMLFile(): Invalid file source %s
27+
--EXPECT--
28+
DOMDocument::loadHTMLFile(): Argument #1 ($filename) must not be empty
29+
DOMDocument::loadHTMLFile(): Argument #1 ($filename) must not contain any null bytes

‎ext/dom/tests/DOMDocument_loadXML_error6.phpt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
Test DOMDocument::loadXML() with empty file path
3+
--SKIPIF--
4+
<?php require_once('skipif.inc'); ?>
5+
--FILE--
6+
<?php
7+
// create dom document
8+
$dom = new DOMDocument();
9+
try {
10+
$dom->loadXML("");
11+
} catch (ValueError $exception) {
12+
echo $exception->getMessage() . "\n";
13+
}
14+
?>
15+
--EXPECT--
16+
DOMDocument::loadXML(): Argument #1 ($source) must not be empty

‎ext/dom/tests/DOMDocument_load_error6.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Test DOMDocument::load() with invalid paths
3+
--SKIPIF--
4+
<?php require_once('skipif.inc'); ?>
5+
--FILE--
6+
<?php
7+
// create dom document
8+
$dom = new DOMDocument();
9+
try {
10+
$dom->load("");
11+
} catch (ValueError $exception) {
12+
echo $exception->getMessage() . "\n";
13+
}
14+
15+
try {
16+
$dom->load("/path/with/\0/byte");
17+
} catch (ValueError $exception) {
18+
echo $exception->getMessage() . "\n";
19+
}
20+
21+
// Path is too long
22+
var_dump($dom->load(str_repeat(" ", PHP_MAXPATHLEN + 1)));
23+
?>
24+
--EXPECT--
25+
DOMDocument::load(): Argument #1 ($filename) must not be empty
26+
Path to document must not contain any null bytes
27+
bool(false)

‎ext/dom/tests/DOMDocument_saveHTMLFile_invalid_filename.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ $title = $doc->createElement('title');
1919
$title = $head->appendChild($title);
2020
$text = $doc->createTextNode('This is the title');
2121
$text = $title->appendChild($text);
22-
$bytes = $doc->saveHTMLFile($filename);
22+
try {
23+
$doc->saveHTMLFile($filename);
24+
} catch (ValueError $exception) {
25+
echo $exception->getMessage() . "\n";
26+
}
2327
?>
24-
--EXPECTF--
25-
Warning: DOMDocument::saveHTMLFile(): Invalid Filename in %s on line %d
28+
--EXPECT--
29+
DOMDocument::saveHTMLFile(): Argument #1 ($filename) must not be empty

‎ext/dom/tests/DOMDocument_schemaValidateSource_error3.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ $doc = new DOMDocument;
1212

1313
$doc->load(__DIR__."/book.xml");
1414

15-
$result = $doc->schemaValidateSource('');
16-
var_dump($result);
15+
try {
16+
$doc->schemaValidateSource('');
17+
} catch (ValueError $e) {
18+
echo $e->getMessage() . "\n";
19+
}
1720

1821
?>
19-
--EXPECTF--
20-
Warning: DOMDocument::schemaValidateSource(): Invalid Schema source in %s.php on line %d
21-
bool(false)
22+
--EXPECT--
23+
DOMDocument::schemaValidateSource(): Argument #1 ($source) must not be empty

‎ext/dom/tests/DOMDocument_schemaValidate_error3.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ $doc = new DOMDocument;
1212

1313
$doc->load(__DIR__."/book.xml");
1414

15-
$result = $doc->schemaValidate('');
16-
var_dump($result);
15+
try {
16+
$doc->schemaValidate('');
17+
} catch (ValueError $e) {
18+
echo $e->getMessage() . "\n";
19+
}
1720

1821
?>
19-
--EXPECTF--
20-
Warning: DOMDocument::schemaValidate(): Invalid Schema source in %s.php on line %d
21-
bool(false)
22+
--EXPECT--
23+
DOMDocument::schemaValidate(): Argument #1 ($filename) must not be empty

‎ext/dom/tests/DOMDocument_schemaValidate_error6.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
DomDocument::schemaValidate() - invalid path to schema
3+
--SKIPIF--
4+
<?php require_once('skipif.inc'); ?>
5+
--FILE--
6+
<?php
7+
8+
$doc = new DOMDocument;
9+
10+
$doc->load(__DIR__."/book.xml");
11+
12+
try {
13+
$doc->schemaValidate("/path/with/\0/byte");
14+
} catch (ValueError $e) {
15+
echo $e->getMessage() . "\n";
16+
}
17+
18+
var_dump($doc->schemaValidate(str_repeat(" ", PHP_MAXPATHLEN + 1)));
19+
20+
?>
21+
--EXPECTF--
22+
DOMDocument::schemaValidate(): Argument #1 ($filename) must not contain any null bytes
23+
24+
Warning: DOMDocument::schemaValidate(): Invalid Schema file source in %s on line %d
25+
bool(false)

‎ext/fileinfo/fileinfo.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ static void _php_finfo_get_type(INTERNAL_FUNCTION_PARAMETERS, int mode, int mime
444444
goto clean;
445445
}
446446
if (CHECK_NULL_PATH(buffer, buffer_len)) {
447-
zend_argument_type_error(1, "must not contain null bytes");
447+
zend_argument_type_error(1, "must not contain any null bytes");
448448
goto clean;
449449
}
450450

‎ext/fileinfo/tests/finfo_file_001.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ var_dump(finfo_file($fp, '&'));
2626

2727
?>
2828
--EXPECTF--
29-
finfo_file(): Argument #1 ($finfo) must not contain null bytes
29+
finfo_file(): Argument #1 ($finfo) must not contain any null bytes
3030
finfo_file(): Argument #1 ($finfo) cannot be empty
3131
finfo_file(): Argument #1 ($finfo) cannot be empty
3232
string(9) "directory"

‎ext/fileinfo/tests/finfo_file_basic.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@ try {
2525
string(28) "text/x-php; charset=us-ascii"
2626
string(22) "PHP script, ASCII text"
2727
string(25) "text/plain; charset=utf-8"
28-
finfo_file(): Argument #1 ($finfo) must not contain null bytes
28+
finfo_file(): Argument #1 ($finfo) must not contain any null bytes

‎ext/fileinfo/tests/mime_content_type_001.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,4 @@ mime_content_type(): Argument #2 must be of type resource|string, array given
4848

4949
Warning: mime_content_type(foo/inexistent): Failed to open stream: No such file or directory in %s on line %d
5050
mime_content_type(): Argument #1 ($filename) cannot be empty
51-
mime_content_type(): Argument #1 ($filename) must not contain null bytes
51+
mime_content_type(): Argument #1 ($filename) must not contain any null bytes

‎ext/sqlite3/sqlite3.c

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,12 +1243,7 @@ PHP_METHOD(SQLite3, openBlob)
12431243

12441244
db_obj = Z_SQLITE3_DB_P(object);
12451245

1246-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "ssl|sl", &table, &table_len, &column, &column_len, &rowid, &dbname, &dbname_len, &flags) == FAILURE) {
1247-
RETURN_THROWS();
1248-
}
1249-
1250-
if (ZEND_NUM_ARGS() >= 4 && CHECK_NULL_PATH(dbname, dbname_len)) {
1251-
zend_argument_type_error(4, "must not contain null bytes");
1246+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "ssl|pl", &table, &table_len, &column, &column_len, &rowid, &dbname, &dbname_len, &flags) == FAILURE) {
12521247
RETURN_THROWS();
12531248
}
12541249

@@ -1346,17 +1341,7 @@ PHP_METHOD(SQLite3, backup)
13461341
sqlite3_backup *dbBackup;
13471342
int rc; // Return code
13481343

1349-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "O|ss", &destination_zval, php_sqlite3_sc_entry, &source_dbname, &source_dbname_length, &destination_dbname, &destination_dbname_length) == FAILURE) {
1350-
RETURN_THROWS();
1351-
}
1352-
1353-
if (ZEND_NUM_ARGS() >= 2 && CHECK_NULL_PATH(source_dbname, source_dbname_length)) {
1354-
zend_argument_type_error(2, "must not contain null bytes");
1355-
RETURN_THROWS();
1356-
}
1357-
1358-
if (ZEND_NUM_ARGS() >= 3 && CHECK_NULL_PATH(destination_dbname, destination_dbname_length)) {
1359-
zend_argument_type_error(3, "must not contain null bytes");
1344+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "O|pp", &destination_zval, php_sqlite3_sc_entry, &source_dbname, &source_dbname_length, &destination_dbname, &destination_dbname_length) == FAILURE) {
13601345
RETURN_THROWS();
13611346
}
13621347

0 commit comments

Comments
 (0)