From 5ac37fbcc6a210de464042185b0fa76228ceae04 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 7 Sep 2021 14:16:00 +0200 Subject: [PATCH 1/8] Fix #61700: FILTER_FLAG_IPV6/FILTER_FLAG_NO_PRIV|RES_RANGE failing It makes no sense to compare IPv6 address ranges as strings; there are too many different representation possibilities. Instead, we change `_php_filter_validate_ipv6()` so that it can calculate the IP address as integer array. We do not rely on `inet_pton()` which may not be available everywhere, at least IPv6 support may not, but rather parse the IP address manually. Finally, we compare the integers. Note that this patch does not fix what we consider as reserved and private, respectively, but merely tries to keep what we had so far. --- ext/filter/logical_filters.c | 105 ++++++++++++++++++--------------- ext/filter/tests/bug47435.phpt | 6 +- ext/filter/tests/bug61700.phpt | 16 +++++ 3 files changed, 77 insertions(+), 50 deletions(-) create mode 100644 ext/filter/tests/bug61700.phpt diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index 1ffc467b43c16..bbea0fbd6d3ee 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -91,7 +91,7 @@ #define FORMAT_IPV4 4 #define FORMAT_IPV6 6 -static int _php_filter_validate_ipv6(char *str, size_t str_len); +static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]); static int php_filter_parse_int(const char *str, size_t str_len, zend_long *ret) { /* {{{ */ zend_long ctx_value; @@ -609,7 +609,7 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ t = e - 1; /* An IPv6 enclosed by square brackets is a valid hostname */ - if (*s == '[' && *t == ']' && _php_filter_validate_ipv6((s + 1), l - 2)) { + if (*s == '[' && *t == ']' && _php_filter_validate_ipv6((s + 1), l - 2, NULL)) { php_url_free(url); return; } @@ -749,11 +749,11 @@ static int _php_filter_validate_ipv4(char *str, size_t str_len, int *ip) /* {{{ } /* }}} */ -static int _php_filter_validate_ipv6(char *str, size_t str_len) /* {{{ */ +static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{{ */ { - int compressed = 0; + int compressed_pos = -1; int blocks = 0; - int n; + int num, n, i, j; char *ipv4; char *end; int ip4elm[4]; @@ -796,35 +796,70 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len) /* {{{ */ return 0; } if (*str == ':') { - if (compressed) { + if (compressed_pos >= 0) { return 0; } - blocks++; /* :: means 1 or more 16-bit 0 blocks */ - compressed = 1; - + if (ip && blocks < 8) { + ip[blocks] = -1; + } + compressed_pos = blocks++; /* :: means 1 or more 16-bit 0 blocks */ if (++str == end) { - return (blocks <= 8); + if (blocks > 8) { + return 0; + } else { + goto fixup_ip; + } } } else if ((str - 1) == s) { /* don't allow leading : without another : following */ return 0; } } - n = 0; + num = n = 0; while ((str < end) && ((*str >= '0' && *str <= '9') || (*str >= 'a' && *str <= 'f') || (*str >= 'A' && *str <= 'F'))) { + if (ip) { + if (*str >= '0' && *str <= '9') { + num = 16 * num + (*str - '0'); + } else if (*str >= 'a' && *str <= 'f') { + num = 16 * num + (*str - 'a') + 10; + } else if (*str >= 'A' && *str <= 'F') { + num = 16 * num + (*str - 'A') + 10; + } + } n++; str++; } + if (ip && blocks < 8) { + ip[blocks] = num; + } if (n < 1 || n > 4) { return 0; } if (++blocks > 8) return 0; } - return ((compressed && blocks <= 8) || blocks == 8); + +fixup_ip: + if (ip && ipv4) { + for (i = 0; i < 5; i++) { + ip[i] = 0; + } + ip[i++] = 0xffff; + ip[i++] = 256 * ip4elm[0] + ip4elm[1]; + ip[i++] = 256 * ip4elm[2] + ip4elm[3]; + } else if (ip && compressed_pos >= 0 && blocks < 8) { + for (i = compressed_pos + 1, j = 7; i < blocks; i++, j--) { + ip[j] = ip[i]; + } + for (i = compressed_pos; i <= j; i++) { + ip[i] = 0; + } + } + + return (compressed_pos >= 0 && blocks <= 8) || blocks == 8; } /* }}} */ @@ -835,7 +870,7 @@ void php_filter_validate_ip(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ * allow_ipv4 and allow_ipv6 flags flag are used, then the first dot or * colon determine the format */ - int ip[4]; + int ip[8]; int mode; if (memchr(Z_STRVAL_P(value), ':', Z_STRLEN_P(value))) { @@ -886,49 +921,25 @@ void php_filter_validate_ip(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ case FORMAT_IPV6: { int res = 0; - res = _php_filter_validate_ipv6(Z_STRVAL_P(value), Z_STRLEN_P(value)); + res = _php_filter_validate_ipv6(Z_STRVAL_P(value), Z_STRLEN_P(value), ip); if (res < 1) { RETURN_VALIDATION_FAILED } /* Check flags */ if (flags & FILTER_FLAG_NO_PRIV_RANGE) { - if (Z_STRLEN_P(value) >=2 && (!strncasecmp("FC", Z_STRVAL_P(value), 2) || !strncasecmp("FD", Z_STRVAL_P(value), 2))) { + if (ip[0] >= 0xfc00 && ip[0] <= 0xfdff) { RETURN_VALIDATION_FAILED } } if (flags & FILTER_FLAG_NO_RES_RANGE) { - switch (Z_STRLEN_P(value)) { - case 1: case 0: - break; - case 2: - if (!strcmp("::", Z_STRVAL_P(value))) { - RETURN_VALIDATION_FAILED - } - break; - case 3: - if (!strcmp("::1", Z_STRVAL_P(value)) || !strcmp("5f:", Z_STRVAL_P(value))) { - RETURN_VALIDATION_FAILED - } - break; - default: - if (Z_STRLEN_P(value) >= 5) { - if ( - !strncasecmp("fe8", Z_STRVAL_P(value), 3) || - !strncasecmp("fe9", Z_STRVAL_P(value), 3) || - !strncasecmp("fea", Z_STRVAL_P(value), 3) || - !strncasecmp("feb", Z_STRVAL_P(value), 3) - ) { - RETURN_VALIDATION_FAILED - } - } - if ( - (Z_STRLEN_P(value) >= 9 && !strncasecmp("2001:0db8", Z_STRVAL_P(value), 9)) || - (Z_STRLEN_P(value) >= 2 && !strncasecmp("5f", Z_STRVAL_P(value), 2)) || - (Z_STRLEN_P(value) >= 4 && !strncasecmp("3ff3", Z_STRVAL_P(value), 4)) || - (Z_STRLEN_P(value) >= 8 && !strncasecmp("2001:001", Z_STRVAL_P(value), 8)) - ) { - RETURN_VALIDATION_FAILED - } + if ((ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 + && ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && (ip[7] == 0 || ip[7] == 1)) + || (ip[0] == 0x5f) + || (ip[0] >= 0xfe80 && ip[0] <= 0xfebf) + || (ip[0] == 0x2001 && (ip[1] == 0x0db8) || (ip[1] >= 0x0010 && ip[1] <= 0x001f)) + || (ip[0] == 0x3ff3) + ) { + RETURN_VALIDATION_FAILED } } } diff --git a/ext/filter/tests/bug47435.phpt b/ext/filter/tests/bug47435.phpt index f192c76a0bf24..e17142aad963d 100644 --- a/ext/filter/tests/bug47435.phpt +++ b/ext/filter/tests/bug47435.phpt @@ -10,8 +10,8 @@ var_dump(filter_var("::", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)); var_dump(filter_var("::", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 | FILTER_FLAG_NO_RES_RANGE)); var_dump(filter_var("::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)); var_dump(filter_var("::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 | FILTER_FLAG_NO_RES_RANGE)); -var_dump(filter_var("fe8:5:6::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)); -var_dump(filter_var("fe8:5:6::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 | FILTER_FLAG_NO_RES_RANGE)); +var_dump(filter_var("fe80:5:6::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)); +var_dump(filter_var("fe80:5:6::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 | FILTER_FLAG_NO_RES_RANGE)); var_dump(filter_var("2001:0db8::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)); var_dump(filter_var("2001:0db8::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 | FILTER_FLAG_NO_RES_RANGE)); var_dump(filter_var("5f::1", FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)); @@ -26,7 +26,7 @@ string(2) "::" bool(false) string(3) "::1" bool(false) -string(10) "fe8:5:6::1" +string(11) "fe80:5:6::1" bool(false) string(12) "2001:0db8::1" bool(false) diff --git a/ext/filter/tests/bug61700.phpt b/ext/filter/tests/bug61700.phpt new file mode 100644 index 0000000000000..8174514f26c0e --- /dev/null +++ b/ext/filter/tests/bug61700.phpt @@ -0,0 +1,16 @@ +--TEST-- +Bug #61700 (FILTER_FLAG_IPV6/FILTER_FLAG_NO_PRIV|RES_RANGE failing) +--SKIPIF-- + +--FILE-- + +--EXPECT-- +bool(false) +string(18) "::ffff:192.168.1.1" +bool(false) From 71601cfbad1ae8eedd7aaec38cff68b18c73a1db Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 7 Sep 2021 14:35:52 +0200 Subject: [PATCH 2/8] Fix misplaced parenthesis --- 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 bbea0fbd6d3ee..9f216be6519e3 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -936,7 +936,7 @@ void php_filter_validate_ip(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ && ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && (ip[7] == 0 || ip[7] == 1)) || (ip[0] == 0x5f) || (ip[0] >= 0xfe80 && ip[0] <= 0xfebf) - || (ip[0] == 0x2001 && (ip[1] == 0x0db8) || (ip[1] >= 0x0010 && ip[1] <= 0x001f)) + || ((ip[0] == 0x2001 && ip[1] == 0x0db8) || (ip[1] >= 0x0010 && ip[1] <= 0x001f)) || (ip[0] == 0x3ff3) ) { RETURN_VALIDATION_FAILED From 7ed1a6316ef1eb6c6aeb96275a974fc7ed00b24b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Sep 2021 12:19:11 +0200 Subject: [PATCH 3/8] No else after return --- ext/filter/logical_filters.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index 9f216be6519e3..2a62018e9fc24 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -806,9 +806,8 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ if (++str == end) { if (blocks > 8) { return 0; - } else { - goto fixup_ip; } + goto fixup_ip; } } else if ((str - 1) == s) { /* don't allow leading : without another : following */ From f300286f0c59d03c33035f7f8826bd330e83af02 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Sep 2021 12:25:12 +0200 Subject: [PATCH 4/8] Avoid duplicated conditionals Co-authored-by: Nikita Popov --- ext/filter/logical_filters.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index 2a62018e9fc24..adf722a9a9f84 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -815,18 +815,15 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ } } num = n = 0; - while ((str < end) && - ((*str >= '0' && *str <= '9') || - (*str >= 'a' && *str <= 'f') || - (*str >= 'A' && *str <= 'F'))) { - if (ip) { - if (*str >= '0' && *str <= '9') { - num = 16 * num + (*str - '0'); - } else if (*str >= 'a' && *str <= 'f') { - num = 16 * num + (*str - 'a') + 10; - } else if (*str >= 'A' && *str <= 'F') { - num = 16 * num + (*str - 'A') + 10; - } + while (str < end) { + if (*str >= '0' && *str <= '9') { + num = 16 * num + (*str - '0'); + } else if (*str >= 'a' && *str <= 'f') { + num = 16 * num + (*str - 'a') + 10; + } else if (*str >= 'A' && *str <= 'F') { + num = 16 * num + (*str - 'A') + 10; + } else { + break; } n++; str++; From 2c7569b6962b948b0d85bb7050b8621b64a27997 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Sep 2021 13:07:18 +0200 Subject: [PATCH 5/8] Properly shift trailing blocks to the end of the array We also need to do this for `blocks == 8`, to replace the single `-1` with `0`. In this case the first `for` loop is unnecessary, but it doesn't really hurt to run it. --- ext/filter/logical_filters.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index adf722a9a9f84..56e964c70f383 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -753,7 +753,7 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ { int compressed_pos = -1; int blocks = 0; - int num, n, i, j; + int num, n, i; char *ipv4; char *end; int ip4elm[4]; @@ -846,11 +846,12 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ ip[i++] = 0xffff; ip[i++] = 256 * ip4elm[0] + ip4elm[1]; ip[i++] = 256 * ip4elm[2] + ip4elm[3]; - } else if (ip && compressed_pos >= 0 && blocks < 8) { - for (i = compressed_pos + 1, j = 7; i < blocks; i++, j--) { - ip[j] = ip[i]; + } else if (ip && compressed_pos >= 0 && blocks <= 8) { + int offset = 8 - blocks; + for (i = 7; i > compressed_pos + 1; i--) { + ip[i] = ip[i - offset]; } - for (i = compressed_pos; i <= j; i++) { + for (i = compressed_pos + offset; i >= compressed_pos; i--) { ip[i] = 0; } } From d69986dedfaa9477e981194d024e86ad0ffe7d81 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Sep 2021 14:38:37 +0200 Subject: [PATCH 6/8] Drop confusing micro optimization --- 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 56e964c70f383..0b820d223c3b0 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -848,7 +848,7 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ ip[i++] = 256 * ip4elm[2] + ip4elm[3]; } else if (ip && compressed_pos >= 0 && blocks <= 8) { int offset = 8 - blocks; - for (i = 7; i > compressed_pos + 1; i--) { + for (i = 7; i > compressed_pos; i--) { ip[i] = ip[i - offset]; } for (i = compressed_pos + offset; i >= compressed_pos; i--) { From de1189590fb566df1670e93c0f80bc1d01cf648d Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Sep 2021 15:55:31 +0200 Subject: [PATCH 7/8] Avoid unnecessary copying --- ext/filter/logical_filters.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index 0b820d223c3b0..378e964d9f618 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -848,9 +848,13 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ ip[i++] = 256 * ip4elm[2] + ip4elm[3]; } else if (ip && compressed_pos >= 0 && blocks <= 8) { int offset = 8 - blocks; - for (i = 7; i > compressed_pos; i--) { + int blocks_to_copy = blocks - compressed_pos - 1; + ZEND_ASSERT(blocks_to_copy >= 0); + /* copy blocks after compressed_pos + 1 to end of array */ + for (i = 7; i > 7 - blocks_to_copy; i--) { ip[i] = ip[i - offset]; } + /* zero compressed blocks */ for (i = compressed_pos + offset; i >= compressed_pos; i--) { ip[i] = 0; } From aa3d7ab201704c70705aa79f8dc8ffc7e25e692a Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 20 Sep 2021 16:19:21 +0200 Subject: [PATCH 8/8] More lucid and simple solution --- ext/filter/logical_filters.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index 378e964d9f618..fa6ae65ac5868 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -848,13 +848,9 @@ static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]) /* {{ ip[i++] = 256 * ip4elm[2] + ip4elm[3]; } else if (ip && compressed_pos >= 0 && blocks <= 8) { int offset = 8 - blocks; - int blocks_to_copy = blocks - compressed_pos - 1; - ZEND_ASSERT(blocks_to_copy >= 0); - /* copy blocks after compressed_pos + 1 to end of array */ - for (i = 7; i > 7 - blocks_to_copy; i--) { + for (i = 7; i > compressed_pos + offset; i--) { ip[i] = ip[i - offset]; } - /* zero compressed blocks */ for (i = compressed_pos + offset; i >= compressed_pos; i--) { ip[i] = 0; }