Skip to content

Commit 9926710

Browse files
author
Dik Takken
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.
1 parent 3d14880 commit 9926710

File tree

7 files changed

+19
-31
lines changed

7 files changed

+19
-31
lines changed

ext/dom/document.c

Lines changed: 7 additions & 6 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);
@@ -1633,8 +1634,8 @@ static void _dom_document_schema_validate(INTERNAL_FUNCTION_PARAMETERS, int type
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) {
@@ -1734,8 +1735,8 @@ static void _dom_document_relaxNG_validate(INTERNAL_FUNCTION_PARAMETERS, int typ
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) {
@@ -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 {

ext/dom/tests/DOMDocument_loadHTMLfile_error2.phpt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ $doc = new DOMDocument();
1414
$result = $doc->loadHTMLFile("");
1515
assert($result === false);
1616
$doc = new DOMDocument();
17-
$result = $doc->loadHTMLFile("text.html\0something");
18-
assert($result === false);
17+
try {
18+
$result = $doc->loadHTMLFile("text.html\0something");
19+
} catch (ValueError $e) {
20+
echo $e->getMessage() . "\n";
21+
}
1922
?>
2023
--EXPECTF--
2124
%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
25+
DOMDocument::loadHTMLFile(): Argument #1 ($filename) must not contain any null bytes

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)