From acadcd6a81bf11aacdc123fd241a5653c0f3605d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 22 May 2024 22:25:02 +0200 Subject: [PATCH 1/2] Fix GHSA-w8qr-v226-r27w We should not early-out with success status if we found an ipv6 hostname, we should keep checking the rest of the conditions. Because integrating the if-check of the ipv6 hostname in the "Validate domain" if-check made the code hard to read, I extracted the condition out to a separate function. This also required to make a few pointers const in order to have some clean code. (cherry picked from commit 4066610b47e22c24cbee91be434a94357056a479) (cherry picked from commit 08be64e40197fc12dca5f802d16748d9c3cb4cb4) (cherry picked from commit 76362f9526afbd5565003d981f9507aaf62337f2) (cherry picked from commit 87a7b8dab75e221a1fcd74cf34dc650f7253c12c) (cherry picked from commit b919ad0323dbc3e1e1ac0f6ba8ec2ad380579918) (cherry picked from commit f232d87846394315071ae115fcb8f1c4d1771eb3) (cherry picked from commit 237878338b1cee3119e3f6d62c640b8cdb6d1169) --- ext/filter/logical_filters.c | 93 +++++++++++++++++++---- ext/filter/tests/ghsa-w8qr-v226-r27w.phpt | 41 ++++++++++ 2 files changed, 119 insertions(+), 15 deletions(-) create mode 100644 ext/filter/tests/ghsa-w8qr-v226-r27w.phpt diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index 92a37f88ee..39260f3622 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -66,6 +66,8 @@ #define FORMAT_IPV4 4 #define FORMAT_IPV6 6 +static int _php_filter_validate_ipv6(const char *str, int str_len TSRMLS_DC); + static int php_filter_parse_int(const char *str, unsigned int str_len, long *ret TSRMLS_DC) { /* {{{ */ long ctx_value; int sign = 0, digit = 0; @@ -445,6 +447,58 @@ void php_filter_validate_regexp(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ } /* }}} */ +static int _php_filter_validate_domain(char * domain, int len) /* {{{ */ +{ + char *e, *s, *t; + size_t l; + int hostname = 1; + unsigned char i = 1; + + s = domain; + l = len; + e = domain + l; + t = e - 1; + + /* Ignore trailing dot */ + if (*t == '.') { + e = t; + l--; + } + + /* The total length cannot exceed 253 characters (final dot not included) */ + if (l > 253) { + return 0; + } + + /* First char must be alphanumeric */ + if(*s == '.' || (hostname && !isalnum((int)*(unsigned char *)s))) { + return 0; + } + + while (s < e) { + if (*s == '.') { + /* The first and the last character of a label must be alphanumeric */ + if (*(s + 1) == '.' || (hostname && (!isalnum((int)*(unsigned char *)(s - 1)) || !isalnum((int)*(unsigned char *)(s + 1))))) { + return 0; + } + + /* Reset label length counter */ + i = 1; + } else { + if (i > 63 || (hostname && *s != '-' && !isalnum((int)*(unsigned char *)s))) { + return 0; + } + + i++; + } + + s++; + } + + return 1; +} +/* }}} */ + static int is_userinfo_valid(char *str) { const char *valid = "-._~!$&'()*+,;=:"; @@ -463,6 +517,14 @@ static int is_userinfo_valid(char *str) return 1; } +static zend_bool php_filter_is_valid_ipv6_hostname(const char *s, size_t l TSRMLS_DC) +{ + const char *e = s + l; + const char *t = e - 1; + + return *s == '[' && *t == ']' && _php_filter_validate_ipv6(s + 1, l - 2 TSRMLS_CC); +} + void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ { php_url *url; @@ -482,25 +544,26 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ } if (url->scheme != NULL && (!strcasecmp(url->scheme, "http") || !strcasecmp(url->scheme, "https"))) { - char *e, *s; + const char *s; + size_t l; if (url->host == NULL) { goto bad_url; } - e = url->host + strlen(url->host); s = url->host; + l = strlen(s); - /* First char of hostname must be alphanumeric */ - if(!isalnum((int)*(unsigned char *)s)) { - goto bad_url; - } + if ( + /* An IPv6 enclosed by square brackets is a valid hostname.*/ + !php_filter_is_valid_ipv6_hostname(s, l TSRMLS_CC) && + /* Validate domain. + * This includes a loose check for an IPv4 address. */ + !_php_filter_validate_domain(url->host, l) - while (s < e) { - if (!isalnum((int)*(unsigned char *)s) && *s != '-' && *s != '.') { - goto bad_url; - } - s++; + ) { + php_url_free(url); + RETURN_VALIDATION_FAILED } } @@ -581,7 +644,7 @@ void php_filter_validate_email(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ } /* }}} */ -static int _php_filter_validate_ipv4(char *str, int str_len, int *ip) /* {{{ */ +static int _php_filter_validate_ipv4(const char *str, int str_len, int *ip) /* {{{ */ { const char *end = str + str_len; int num, m; @@ -616,15 +679,15 @@ static int _php_filter_validate_ipv4(char *str, int str_len, int *ip) /* {{{ */ } /* }}} */ -static int _php_filter_validate_ipv6(char *str, int str_len TSRMLS_DC) /* {{{ */ +static int _php_filter_validate_ipv6(const char *str, int str_len TSRMLS_DC) /* {{{ */ { int compressed = 0; int blocks = 0; int n; char *ipv4; - char *end; + const char *end; int ip4elm[4]; - char *s = str; + const char *s = str; if (!memchr(str, ':', str_len)) { return 0; diff --git a/ext/filter/tests/ghsa-w8qr-v226-r27w.phpt b/ext/filter/tests/ghsa-w8qr-v226-r27w.phpt new file mode 100644 index 0000000000..97867d4b07 --- /dev/null +++ b/ext/filter/tests/ghsa-w8qr-v226-r27w.phpt @@ -0,0 +1,41 @@ +--TEST-- +GHSA-w8qr-v226-r27w +--EXTENSIONS-- +filter +--FILE-- + +--EXPECT-- +--- These ones should fail --- +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +--- These ones should work --- +string(21) "http://test@127.0.0.1" +string(50) "http://test@[2001:db8:3333:4444:5555:6666:1.2.3.4]" +string(17) "http://test@[::1]" -- 2.45.1 From aa6e34b7cb4ea37d5f7cab27a560df8f0b763908 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Tue, 4 Jun 2024 16:48:08 +0200 Subject: [PATCH 2/2] NEWS (cherry picked from commit a1ff81b786bd519597e770795be114f5171f0648) (cherry picked from commit ec1d5e6468479e64acc7fb8cb955f053b64ea9a0) (cherry picked from commit cfe1b1acead13b6af163f3ce947d3a1dbded82a0) (cherry picked from commit 6b6444a1b72d6249cfa592f20395efe67ca55f73) (cherry picked from commit 8d4db37794eff4761a336d7cee53d47f0eb0d313) (cherry picked from commit 573b921a612068f66f6540b69b0a9bc9f372ecf1) (cherry picked from commit 20ceeb85afad56df396a61247f2cb4cb9efb9a6b) --- NEWS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS b/NEWS index 163bc6bdba..c10febbfa1 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,12 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| +Backported from 8.1.29 + +- Filter: + . Fixed bug GHSA-w8qr-v226-r27w (Filter bypass in filter_var FILTER_VALIDATE_URL). + (CVE-2024-5458) (nielsdos) + Backported from 8.1.28 - Standard: -- 2.45.1