From ad56ec7bbfd06ddff0739de5a8a58bcd37685a35 Mon Sep 17 00:00:00 2001 From: Derick Rethans Date: Wed, 5 Jun 2024 16:47:06 +0100 Subject: [PATCH 1/3] Fixed off-by-one error in checking length of abtract namespace Unix sockets --- .../{bug60106.phpt => bug60106-001.phpt} | 6 +-- ext/standard/tests/streams/bug60106-002.phpt | 40 +++++++++++++++++++ main/streams/xp_socket.c | 11 +++-- 3 files changed, 51 insertions(+), 6 deletions(-) rename ext/standard/tests/streams/{bug60106.phpt => bug60106-001.phpt} (78%) create mode 100644 ext/standard/tests/streams/bug60106-002.phpt diff --git a/ext/standard/tests/streams/bug60106.phpt b/ext/standard/tests/streams/bug60106-001.phpt similarity index 78% rename from ext/standard/tests/streams/bug60106.phpt rename to ext/standard/tests/streams/bug60106-001.phpt index 1b36af1a3d9..82c0d94a734 100644 --- a/ext/standard/tests/streams/bug60106.phpt +++ b/ext/standard/tests/streams/bug60106-001.phpt @@ -1,5 +1,5 @@ --TEST-- -Bug#60106 (stream_socket_server silently truncates long unix socket paths) +Bug #60106 (stream_socket_server silently truncates long unix socket paths) --SKIPIF-- +--FILE-- + +--EXPECTF-- +stream_socket_server(): socket path exceeded the maximum allowed length of %d bytes and was truncated +stream_socket_server(): socket path exceeded the maximum allowed length of %d bytes and was truncated +Allowed length is now one more: yes diff --git a/main/streams/xp_socket.c b/main/streams/xp_socket.c index 3ba79df6349..9987f871a7a 100644 --- a/main/streams/xp_socket.c +++ b/main/streams/xp_socket.c @@ -564,18 +564,23 @@ static inline int parse_unix_address(php_stream_xport_param *xparam, struct sock memset(unix_addr, 0, sizeof(*unix_addr)); unix_addr->sun_family = AF_UNIX; + /* Abstract namespace does not need to be NUL-terminated, while path-based + * sockets should be. */ + bool is_abstract_ns = xparam->inputs.namelen > 0 && xparam->inputs.name[0] == '\0'; + unsigned long max_length = is_abstract_ns ? sizeof(unix_addr->sun_path) : sizeof(unix_addr->sun_path) - 1; + /* we need to be binary safe on systems that support an abstract * namespace */ - if (xparam->inputs.namelen >= sizeof(unix_addr->sun_path)) { + if (xparam->inputs.namelen > max_length) { /* On linux, when the path begins with a NUL byte we are * referring to an abstract namespace. In theory we should * allow an extra byte below, since we don't need the NULL. * BUT, to get into this branch of code, the name is too long, * so we don't care. */ - xparam->inputs.namelen = sizeof(unix_addr->sun_path) - 1; + xparam->inputs.namelen = max_length; php_error_docref(NULL, E_NOTICE, "socket path exceeded the maximum allowed length of %lu bytes " - "and was truncated", (unsigned long)sizeof(unix_addr->sun_path)); + "and was truncated", max_length); } memcpy(unix_addr->sun_path, xparam->inputs.name, xparam->inputs.namelen); From c595ab96abfc6e76ba9af516f50f9fd9d0fbfdf5 Mon Sep 17 00:00:00 2001 From: Derick Rethans Date: Thu, 13 Jun 2024 14:04:03 +0100 Subject: [PATCH 2/3] Update NEWS --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 9aaa4db7f27..76604c3b66b 100644 --- a/NEWS +++ b/NEWS @@ -60,6 +60,10 @@ PHP NEWS . Fixed bug GH-14290 (Member access within null pointer in extension spl). (nielsdos) +- Standard: + . Fixed bug GH-14483 (Fixed off-by-one error in checking length of abstract + namespace Unix sockets). (Derick) + - Streams: . Fixed bug GH-11078 (PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors). (nielsdos) From 784b745e0758dacc1dd8acfe1abc08eb215b8a15 Mon Sep 17 00:00:00 2001 From: Derick Rethans Date: Thu, 13 Jun 2024 14:04:42 +0100 Subject: [PATCH 3/3] Update NEWS --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index e8a40903bce..fc339dbffb5 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,10 @@ PHP NEWS . Fixed bug GH-14290 (Member access within null pointer in extension spl). (nielsdos) +- Standard: + . Fixed bug GH-14483 (Fixed off-by-one error in checking length of abstract + namespace Unix sockets). (Derick) + - Streams: . Fixed bug GH-11078 (PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors). (nielsdos)