FILTER_FLAG_HOSTNAME accepts ending hyphen

[ppaysant]

Description

Hello,

The following code:

<?php

$domains = [
    'a-.bc.com',
    'a.bc-.com',
    'a.bc.com-',
];

foreach ($domains as $domain) {
    echo $domain . PHP_EOL;
    var_dump(filter_var($domain, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME));
    echo PHP_EOL;
}

Resulted in this output:

a-.bc.com
bool(false)

a.bc-.com
bool(false)

a.bc.com-
string(9) "a.bc.com-"    // <------------ here is the bug

I expected this output instead: a.bc.com- test should return false.
I thought that the ending hyphen is not permitted on domain name (https://datatracker.ietf.org/doc/html/rfc1035#section-2.3.1).

This code is tested here : https://3v4l.org/XbMCV

This bug happens in PHP 8.1.30, PHP 8.2.24 and PHP 8.3.12

PHP Version

PHP 8.3.12

Operating System

[devnexen]

It is fixable, e.g. if we re looking for hostname, the last element after . should not be an hyphen. However implementation of these filters might not be necessarily "turing complete" more or less on purpose, let's see what @Girgias thinks.

[Girgias]

The filter extension is quite old, and I haven't looked much into it. IIRC the original author is @derickr and he might have more of an idea.

[cmb69]

 ext/filter/logical_filters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c
index b875819927..7de2dbb96a 100644
--- a/ext/filter/logical_filters.c
+++ b/ext/filter/logical_filters.c
@@ -542,7 +542,7 @@ static int _php_filter_validate_domain(char * domain, size_t len, zend_long flag
 			/* Reset label length counter */
 			i = 1;
 		} else {
-			if (i > 63 || (hostname && *s != '-' && !isalnum((int)*(unsigned char *)s))) {
+			if (i > 63 || (hostname && (*s != '-' || (*s == '-' && *(s + 1) == '\0')) && !isalnum((int)*(unsigned char *)s))) {
 				return 0;
 			}

[cmb69]

Oops, can be simplified:

 ext/filter/logical_filters.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c
index b875819927..81f44dafc3 100644
--- a/ext/filter/logical_filters.c
+++ b/ext/filter/logical_filters.c
@@ -542,7 +542,7 @@ static int _php_filter_validate_domain(char * domain, size_t len, zend_long flag
 			/* Reset label length counter */
 			i = 1;
 		} else {
-			if (i > 63 || (hostname && *s != '-' && !isalnum((int)*(unsigned char *)s))) {
+			if (i > 63 || (hostname && (*s != '-' || *(s + 1) == '\0') && !isalnum((int)*(unsigned char *)s))) {
 				return 0;
 			}

[devnexen]

Does it work with examples like e-book.com ?

[cmb69]

Yeah, sure. See

php-src/ext/filter/logical_filters.c

Lines 535 to 550 in 65f8857

	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++;
	}

The if branch is for all labels but the last. The else branch is only for the last label, where it had been overlooked to cater to a hyphen at the end.