Skip to content

Commit e95083b

Browse files
committed
uri: Fix lexbor memory management in uri_parser_whatwg.c
Calling `lexbor_mraw_clean()` after a specific number of parses will destroy the data for any live `Uri\WhatWg\Url` objects, effectively resulting in a use-after-free. Fix the issue by removing the periodic `lexbor_mraw_clean()` call. Instead we implement `php_uri_parser_whatwg_free()`. This also requires to move the destruction of the lexbor structures from RSHUTDOWN to POST_ZEND_DEACTIVATE to prevent a use-after-free in `php_uri_parser_whatwg_free()` since otherwise the mraw would already have been destroyed.
1 parent 5d5305d commit e95083b

File tree

4 files changed

+36
-24
lines changed

4 files changed

+36
-24
lines changed

ext/uri/php_uri.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,9 +1084,9 @@ PHP_RINIT_FUNCTION(uri)
10841084
return SUCCESS;
10851085
}
10861086

1087-
PHP_RSHUTDOWN_FUNCTION(uri)
1087+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(uri)
10881088
{
1089-
if (PHP_RSHUTDOWN(uri_parser_whatwg)(INIT_FUNC_ARGS_PASSTHRU) == FAILURE) {
1089+
if (ZEND_MODULE_POST_ZEND_DEACTIVATE_N(uri_parser_whatwg)() == FAILURE) {
10901090
return FAILURE;
10911091
}
10921092

@@ -1101,8 +1101,10 @@ zend_module_entry uri_module_entry = {
11011101
PHP_MINIT(uri), /* PHP_MINIT - Module initialization */
11021102
PHP_MSHUTDOWN(uri), /* PHP_MSHUTDOWN - Module shutdown */
11031103
PHP_RINIT(uri), /* PHP_RINIT - Request initialization */
1104-
PHP_RSHUTDOWN(uri), /* PHP_RSHUTDOWN - Request shutdown */
1104+
NULL, /* PHP_RSHUTDOWN - Request shutdown */
11051105
PHP_MINFO(uri), /* PHP_MINFO - Module info */
11061106
PHP_VERSION, /* Version */
1107-
STANDARD_MODULE_PROPERTIES
1107+
NO_MODULE_GLOBALS,
1108+
ZEND_MODULE_POST_ZEND_DEACTIVATE_N(uri),
1109+
STANDARD_MODULE_PROPERTIES_EX
11081110
};

ext/uri/tests/056.phpt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
Test Lexbor memory management
3+
--EXTENSIONS--
4+
uri
5+
--FILE--
6+
<?php
7+
8+
$urls = [];
9+
10+
for ($i = 0; $i < 1000; $i++) {
11+
$urls[] = Uri\WhatWg\Url::parse("https://example.com/{$i}/");
12+
}
13+
14+
for ($i = 0; $i < 1000; $i++) {
15+
if ($urls[$i]->toAsciiString() !== "https://example.com/{$i}/") {
16+
die("FAIL");
17+
}
18+
}
19+
20+
?>
21+
==DONE==
22+
--EXPECT--
23+
==DONE==

ext/uri/uri_parser_whatwg.c

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@
2424
#include <arpa/inet.h>
2525
#endif
2626

27-
ZEND_TLS lxb_url_parser_t lexbor_parser;
28-
ZEND_TLS unsigned short int parsed_urls;
27+
ZEND_TLS lxb_url_parser_t lexbor_parser = {0};
2928

30-
static const unsigned short int maximum_parses_before_cleanup = 500;
3129
static const size_t lexbor_mraw_byte_size = 8192;
3230

3331
static zend_always_inline void zval_string_or_null_to_lexbor_str(zval *value, lexbor_str_t *lexbor_str)
@@ -539,34 +537,20 @@ PHP_RINIT_FUNCTION(uri_parser_whatwg)
539537
return FAILURE;
540538
}
541539

542-
parsed_urls = 0;
543-
544540
return SUCCESS;
545541
}
546542

547-
PHP_RSHUTDOWN_FUNCTION(uri_parser_whatwg)
543+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(uri_parser_whatwg)
548544
{
549545
lxb_url_parser_memory_destroy(&lexbor_parser);
550546
lxb_url_parser_destroy(&lexbor_parser, false);
551547

552-
parsed_urls = 0;
553-
554548
return SUCCESS;
555549
}
556550

557-
static void reset_parser_state(void)
558-
{
559-
if (++parsed_urls % maximum_parses_before_cleanup == 0) {
560-
lexbor_mraw_clean(lexbor_parser.mraw);
561-
parsed_urls = 0;
562-
}
563-
564-
lxb_url_parser_clean(&lexbor_parser);
565-
}
566-
567551
lxb_url_t *php_uri_parser_whatwg_parse_ex(const char *uri_str, size_t uri_str_len, const lxb_url_t *lexbor_base_url, zval *errors, bool silent)
568552
{
569-
reset_parser_state();
553+
lxb_url_parser_clean(&lexbor_parser);
570554

571555
lxb_url_t *url = lxb_url_parse(&lexbor_parser, lexbor_base_url, (unsigned char *) uri_str, uri_str_len);
572556
const char *reason = fill_errors(errors);
@@ -619,6 +603,9 @@ static zend_string *php_uri_parser_whatwg_to_string(void *uri, uri_recomposition
619603

620604
static void php_uri_parser_whatwg_free(void *uri)
621605
{
606+
lxb_url_t *lexbor_uri = uri;
607+
608+
lxb_url_destroy(lexbor_uri);
622609
}
623610

624611
const uri_parser_t php_uri_parser_whatwg = {

ext/uri/uri_parser_whatwg.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ extern const uri_parser_t php_uri_parser_whatwg;
2525
lxb_url_t *php_uri_parser_whatwg_parse_ex(const char *uri_str, size_t uri_str_len, const lxb_url_t *lexbor_base_url, zval *errors, bool silent);
2626

2727
PHP_RINIT_FUNCTION(uri_parser_whatwg);
28-
PHP_RSHUTDOWN_FUNCTION(uri_parser_whatwg);
28+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(uri_parser_whatwg);
2929

3030
#endif

0 commit comments

Comments
 (0)