summaryrefslogtreecommitdiffstats
path: root/php-cve-2024-5458.patch
blob: 4db4af4558b85b79b636b720ebab06eb39b24b6e (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
From 4066610b47e22c24cbee91be434a94357056a479 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.
---
 ext/filter/logical_filters.c              | 35 ++++++++++---------
 ext/filter/tests/ghsa-w8qr-v226-r27w.phpt | 41 +++++++++++++++++++++++
 2 files changed, 61 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 ad011568aac..300c6e2809c 100644
--- a/ext/filter/logical_filters.c
+++ b/ext/filter/logical_filters.c
@@ -89,7 +89,7 @@
 #define FORMAT_IPV4    4
 #define FORMAT_IPV6    6
 
-static int _php_filter_validate_ipv6(char *str, size_t str_len, int ip[8]);
+static int _php_filter_validate_ipv6(const 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;
@@ -572,6 +572,14 @@ static int is_userinfo_valid(zend_string *str)
 	return 1;
 }
 
+static bool php_filter_is_valid_ipv6_hostname(const char *s, size_t l)
+{
+	const char *e = s + l;
+	const char *t = e - 1;
+
+	return *s == '[' && *t == ']' && _php_filter_validate_ipv6(s + 1, l - 2, NULL);
+}
+
 void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
 {
 	php_url *url;
@@ -592,7 +600,7 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
 
 	if (url->scheme != NULL &&
 		(zend_string_equals_literal_ci(url->scheme, "http") || zend_string_equals_literal_ci(url->scheme, "https"))) {
-		char *e, *s, *t;
+		const char *s;
 		size_t l;
 
 		if (url->host == NULL) {
@@ -601,17 +609,14 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
 
 		s = ZSTR_VAL(url->host);
 		l = ZSTR_LEN(url->host);
-		e = s + l;
-		t = e - 1;
-
-		/* An IPv6 enclosed by square brackets is a valid hostname */
-		if (*s == '[' && *t == ']' && _php_filter_validate_ipv6((s + 1), l - 2, NULL)) {
-			php_url_free(url);
-			return;
-		}
 
-		// Validate domain
-		if (!_php_filter_validate_domain(ZSTR_VAL(url->host), l, FILTER_FLAG_HOSTNAME)) {
+		if (
+			/* An IPv6 enclosed by square brackets is a valid hostname.*/
+			!php_filter_is_valid_ipv6_hostname(s, l) &&
+			/* Validate domain.
+			 * This includes a loose check for an IPv4 address. */
+			!_php_filter_validate_domain(ZSTR_VAL(url->host), l, FILTER_FLAG_HOSTNAME)
+		) {
 			php_url_free(url);
 			RETURN_VALIDATION_FAILED
 		}
@@ -745,15 +750,15 @@ 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, int ip[8]) /* {{{ */
+static int _php_filter_validate_ipv6(const char *str, size_t str_len, int ip[8]) /* {{{ */
 {
 	int compressed_pos = -1;
 	int blocks = 0;
 	int num, n, i;
 	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 00000000000..0092408ee5a
--- /dev/null
+++ b/ext/filter/tests/ghsa-w8qr-v226-r27w.phpt
@@ -0,0 +1,41 @@
+--TEST--
+GHSA-w8qr-v226-r27w
+--EXTENSIONS--
+filter
+--FILE--
+<?php
+
+function test(string $input) {
+    var_dump(filter_var($input, FILTER_VALIDATE_URL));
+}
+
+echo "--- These ones should fail ---\n";
+test("http://t[est@127.0.0.1");
+test("http://t[est@[::1]");
+test("http://t[est@[::1");
+test("http://t[est@::1]");
+test("http://php.net\\@aliyun.com/aaa.do");
+test("http://test[@2001:db8:3333:4444:5555:6666:1.2.3.4]");
+test("http://te[st@2001:db8:3333:4444:5555:6666:1.2.3.4]");
+test("http://te[st@2001:db8:3333:4444:5555:6666:1.2.3.4");
+
+echo "--- These ones should work ---\n";
+test("http://test@127.0.0.1");
+test("http://test@[2001:db8:3333:4444:5555:6666:1.2.3.4]");
+test("http://test@[::1]");
+
+?>
+--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 a1ff81b786bd519597e770795be114f5171f0648 Mon Sep 17 00:00:00 2001
From: Remi Collet <remi@remirepo.net>
Date: Tue, 4 Jun 2024 16:48:08 +0200
Subject: [PATCH 2/2] NEWS

---
 NEWS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/NEWS b/NEWS
index 1300609f189..7a9b6bdae18 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