Skip to content

Commit 7e0e3cc

Browse files
nielsdosramsey
authored andcommitted
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.
1 parent 9382673 commit 7e0e3cc

File tree

2 files changed

+61
-15
lines changed

2 files changed

+61
-15
lines changed

ext/filter/logical_filters.c

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@
8989
#define FORMAT_IPV4 4
9090
#define FORMAT_IPV6 6
9191

92-
static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]);
92+
static int _php_filter_validate_ipv6(const char *str, size_t str_len, int ip[8]);
9393

9494
static int php_filter_parse_int(const char *str, size_t str_len, zend_long *ret) { /* {{{ */
9595
zend_long ctx_value;
@@ -580,6 +580,14 @@ static int is_userinfo_valid(zend_string *str)
580580
return 1;
581581
}
582582

583+
static bool php_filter_is_valid_ipv6_hostname(const char *s, size_t l)
584+
{
585+
const char *e = s + l;
586+
const char *t = e - 1;
587+
588+
return *s == '[' && *t == ']' && _php_filter_validate_ipv6(s + 1, l - 2, NULL);
589+
}
590+
583591
void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
584592
{
585593
php_url *url;
@@ -600,7 +608,7 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
600608

601609
if (url->scheme != NULL &&
602610
(zend_string_equals_literal_ci(url->scheme, "http") || zend_string_equals_literal_ci(url->scheme, "https"))) {
603-
char *e, *s, *t;
611+
const char *s;
604612
size_t l;
605613

606614
if (url->host == NULL) {
@@ -609,17 +617,14 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
609617

610618
s = ZSTR_VAL(url->host);
611619
l = ZSTR_LEN(url->host);
612-
e = s + l;
613-
t = e - 1;
614-
615-
/* An IPv6 enclosed by square brackets is a valid hostname */
616-
if (*s == '[' && *t == ']' && _php_filter_validate_ipv6((s + 1), l - 2, NULL)) {
617-
php_url_free(url);
618-
return;
619-
}
620620

621-
// Validate domain
622-
if (!_php_filter_validate_domain(ZSTR_VAL(url->host), l, FILTER_FLAG_HOSTNAME)) {
621+
if (
622+
/* An IPv6 enclosed by square brackets is a valid hostname.*/
623+
!php_filter_is_valid_ipv6_hostname(s, l) &&
624+
/* Validate domain.
625+
* This includes a loose check for an IPv4 address. */
626+
!_php_filter_validate_domain(ZSTR_VAL(url->host), l, FILTER_FLAG_HOSTNAME)
627+
) {
623628
php_url_free(url);
624629
RETURN_VALIDATION_FAILED
625630
}
@@ -753,15 +758,15 @@ static int _php_filter_validate_ipv4(char *str, size_t str_len, int *ip) /* {{{
753758
}
754759
/* }}} */
755760

756-
static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{{ */
761+
static int _php_filter_validate_ipv6(const char *str, size_t str_len, int ip[8]) /* {{{ */
757762
{
758763
int compressed_pos = -1;
759764
int blocks = 0;
760765
int num, n, i;
761766
char *ipv4;
762-
char *end;
767+
const char *end;
763768
int ip4elm[4];
764-
char *s = str;
769+
const char *s = str;
765770

766771
if (!memchr(str, ':', str_len)) {
767772
return 0;
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
GHSA-w8qr-v226-r27w
3+
--EXTENSIONS--
4+
filter
5+
--FILE--
6+
<?php
7+
8+
function test(string $input) {
9+
var_dump(filter_var($input, FILTER_VALIDATE_URL));
10+
}
11+
12+
echo "--- These ones should fail ---\n";
13+
test("http://t[est@127.0.0.1");
14+
test("http://t[est@[::1]");
15+
test("http://t[est@[::1");
16+
test("http://t[est@::1]");
17+
test("http://php.net\\@aliyun.com/aaa.do");
18+
test("http://test[@2001:db8:3333:4444:5555:6666:1.2.3.4]");
19+
test("http://te[st@2001:db8:3333:4444:5555:6666:1.2.3.4]");
20+
test("http://te[st@2001:db8:3333:4444:5555:6666:1.2.3.4");
21+
22+
echo "--- These ones should work ---\n";
23+
test("http://test@127.0.0.1");
24+
test("http://test@[2001:db8:3333:4444:5555:6666:1.2.3.4]");
25+
test("http://test@[::1]");
26+
27+
?>
28+
--EXPECT--
29+
--- These ones should fail ---
30+
bool(false)
31+
bool(false)
32+
bool(false)
33+
bool(false)
34+
bool(false)
35+
bool(false)
36+
bool(false)
37+
bool(false)
38+
--- These ones should work ---
39+
string(21) "http://test@127.0.0.1"
40+
string(50) "http://test@[2001:db8:3333:4444:5555:6666:1.2.3.4]"
41+
string(17) "http://test@[::1]"

0 commit comments

Comments
 (0)