Bug#890266: php5-common: 0050-Detect-invalid-port-in-xp_socket-parse-ip-address.patch incomplete

2018-03-28 Thread jack
Source: php5
Version: 5.6.33+dfsg-0+deb8u1
Followup-For: Bug #890266

Dear Maintainer,

I confirm the existence of this bug

The following snippet triggers the issue:
stream_socket_client('tcp://localhost:6379/');

On php 5.6.30+dfsg-0+deb8u1, the call is a success
On php 5.6.33+dfsg-0+deb8u1:
PHP Warning:  stream_socket_client(): unable to connect to 
tcp://localhost:6379/ (Failed to parse address "localhost:6379/") in 
/tmp/test.php on line 3

The '/' behavior seems to be a long known trick, used by many libs etc (which 
are broken with the 5.6.33 release), despite the absence of official 
documentation:
https://stackoverflow.com/a/30391981/4091648
https://secure.php.net/manual/en/function.stream-socket-client.php#105393

Thanks for maintaining,

-- System Information:
Debian Release: 9.4
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'oldoldstable'), (500, 'unstable'), (500, 
'oldstable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.9.0-5-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/das



Bug#890266: php5-common: 0050-Detect-invalid-port-in-xp_socket-parse-ip-address.patch incomplete

2018-02-12 Thread jfot
Package: php5-common
Version: 5.6.33+dfsg-0+deb8u1
Severity: important
Tags: patch

There was a bug reported and fixed:
https://bugs.php.net/bug.php?id=74216
https://security-tracker.debian.org/tracker/CVE-2017-7272

The fix consisted of two parts, first:
https://github.com/php/php-src/commit/bab0b99f376dac9170ac81382a5ed526938d595a
the fix had a big impact on applications relying on the "feature"
```
php5 -r 'print fsockopen("tcp://127.0.0.1:80/foo");'
```

Because of the heavy impact, a fixfix was released:
https://github.com/php/php-src/commit/cda7dcf4cacef3346f9dc2a4dc947e6a74769259

Now, security.debian.org offers 5.6.33+dfsg-0+deb8u1 whcih includes the
following patch:
https://sources.debian.org/src/php5/5.6.33+dfsg-0+deb8u1/debian/patches/0050
-Detect-invalid-port-in-xp_socket-parse-ip-address.patch/

But this is only part 1 of the fix. With this patch, the provided call above
doesn't work anymore.
This means we have to hold back the update. Magento(ecommerce) + Redis won't
work with this incomplete patch.

Please consider removing the patch or completing it with the fixfix.



-- System Information:
Debian Release: stretch/sid
  APT prefers xenial-updates
  APT policy: (500, 'xenial-updates'), (500, 'xenial-security'), (500, 
'xenial'), (100, 'xenial-backports')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.10.0-42-generic (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
>From cda7dcf4cacef3346f9dc2a4dc947e6a74769259 Mon Sep 17 00:00:00 2001
From: Sara Golemon 
Date: Tue, 25 Apr 2017 12:52:48 +0200
Subject: [PATCH] Follow up patch regarding bug #74216, see bug #74429

While the case in bug #74429 is not documented and is only worky due to
an implementation bug, the strength seems to breach some real world
apps. Given this patch doesn't impact the initial security fix for
bug #74216, it is reasonable to let the apps keep working. As mentioned
in the ticket, this behavior is a subject to change in future versions
and should not be abused.
---
 main/streams/xp_socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/main/streams/xp_socket.c b/main/streams/xp_socket.c
index 3ff64787aa14..92be33326081 100644
--- a/main/streams/xp_socket.c
+++ b/main/streams/xp_socket.c
@@ -581,7 +581,7 @@ static inline char *parse_ip_address_ex(const char *str, size_t str_len, int *po
 			return NULL;
 		}
 		*portno = strtol(p + 2, , 10);
-		if (e && *e) {
+		if (e && *e && *e != '/') {
 			if (get_err) {
 *err = strpprintf(0, "Failed to parse address \"%s\"", str);
 			}
@@ -600,7 +600,7 @@ static inline char *parse_ip_address_ex(const char *str, size_t str_len, int *po
 	if (colon) {
 		char *e = NULL;
 		*portno = strtol(colon + 1, , 10);
-		if (!e || !*e) {
+		if (!e || !*e || *e == '/') {
 			return estrndup(str, colon - str);
 		}
 	}
>From cda7dcf4cacef3346f9dc2a4dc947e6a74769259 Mon Sep 17 00:00:00 2001
From: Sara Golemon 
Date: Tue, 25 Apr 2017 12:52:48 +0200
Subject: [PATCH] Follow up patch regarding bug #74216, see bug #74429

While the case in bug #74429 is not documented and is only worky due to
an implementation bug, the strength seems to breach some real world
apps. Given this patch doesn't impact the initial security fix for
bug #74216, it is reasonable to let the apps keep working. As mentioned
in the ticket, this behavior is a subject to change in future versions
and should not be abused.
---
 main/streams/xp_socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/main/streams/xp_socket.c b/main/streams/xp_socket.c
index 3ff64787aa14..92be33326081 100644
--- a/main/streams/xp_socket.c
+++ b/main/streams/xp_socket.c
@@ -581,7 +581,7 @@ static inline char *parse_ip_address_ex(const char *str, size_t str_len, int *po
 			return NULL;
 		}
 		*portno = strtol(p + 2, , 10);
-		if (e && *e) {
+		if (e && *e && *e != '/') {
 			if (get_err) {
 *err = strpprintf(0, "Failed to parse address \"%s\"", str);
 			}
@@ -600,7 +600,7 @@ static inline char *parse_ip_address_ex(const char *str, size_t str_len, int *po
 	if (colon) {
 		char *e = NULL;
 		*portno = strtol(colon + 1, , 10);
-		if (!e || !*e) {
+		if (!e || !*e || *e == '/') {
 			return estrndup(str, colon - str);
 		}
 	}
>From cda7dcf4cacef3346f9dc2a4dc947e6a74769259 Mon Sep 17 00:00:00 2001
From: Sara Golemon 
Date: Tue, 25 Apr 2017 12:52:48 +0200
Subject: [PATCH] Follow up patch regarding bug #74216, see bug #74429

While the case in bug #74429 is not documented and is only worky due to
an implementation bug, the strength seems to breach some real world
apps. Given this patch doesn't impact the initial security fix for
bug #74216, it is reasonable to let the apps keep working. As mentioned
in the ticket, this behavior is a subject to change in future versions
and should not be abused.
---