On Wed, Oct 28, 2026 at 07:46:44AM -0700, Martin Mirchev wrote:
> Hi,
> Here are the patches for GitHub issues #3545 and #3546, as Amaury
> suggested in #3547, against current master (8c84ab562). They are attached
> (git format-patch) and independent of each other.
> 1/2 BUG/MINOR: sample: fix NULL dereference in the ltime/utime converters
> conv_time_common() returns NULL when the expanded format does not fit
> in the trash chunk or when alloc_trash_chunk() fails, and its four
> callers dereference it. With ms_ltime("%N" x 3000) the config is
> accepted and the first request crashes the process. The converters
> now fail instead. Backport to 2.9 and later.Thanks, I merged this patch. I only slightly edited the commit message to correct the backport reference to 3.1 as this version is end of life. > 2/2 BUG/MINOR: debug: fix "debug dev fd" crash on unknown address families > sa2str() returns NULL for families other than AF_INET, AF_INET6 and > AF_UNIX, and "debug dev fd" passes it to strlen(). With an inherited > AF_NETLINK socket the command crashes the process. It now prints "?". > Backport to 3.1 and later. > Tested on 8c84ab562, release build (USE_OPENSSL=1): on master all four > converters (ms_ltime, us_ltime, ms_utime, us_utime) and "debug dev fd" with > an inherited netlink socket crash with SIGSEGV; with the patches each > converter request returns 200, "debug dev fd" prints "laddr=? raddr=?" for > the netlink fd, and the process keeps running. > Both issues were found with a static analysis engine (Infer/Pulse) and > verified manually. Regarding this patch, there is several points that can be improved. In my opinion the code is simpler with a single NULL check around both the cleanup loop and the dump. Also, the commit message is not totally accurate. First, I removed the reference to AF_NETLINK as I'm not sure haproxy is really dealing with such sockets. Then, sa2str() can also return NULL due to a memory allocation failure, so a crash could happen even with supported address families. Finally, regarding the backport, I think it should be picked up to 2.6, as the original "debug dev fd" handler already contains the dump of a possible NULL value. I have written a new version for the "debug dev fd" fix, attached as a patch with this email. Please tell me if this suits you. Thanks for your contribution, -- Amaury Denoyelle
>From 81bf7ffcc4588e503647ce4aff3189cbff048e91 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle <[email protected]> Date: Fri, 30 Oct 2026 10:53:08 +0100 Subject: [PATCH] BUG/MINOR: debug: fix "debug dev fd" crash on unknown address families "debug dev fd" formats the local and peer addresses of each fd with sa2str() and then replaces non-printable characters in the result. It returns NULL for address families other than AF_INET, AF_INET6, AF_UNIX and some AF_CUST_*. For supported address families, it can also return NULL in case of a memory allocation failure. Only an admin in expert mode can run the command. Let's skip the cleanup and print "?" when sa2str() fails. This fixes GitHub issue #3546. This must be backported to all stable versions. In fact, strlen() without guard was introduced in 3.1, which guarantees a crash in case of an unsupported address. For older releases, only an unsafe chunk_appendf() is present which may cause issues. --- src/debug.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/debug.c b/src/debug.c index bc6355e12..69c1383b6 100644 --- a/src/debug.c +++ b/src/debug.c @@ -2304,13 +2304,18 @@ static int debug_iohandler_fd(struct appctx *appctx) port = 0; addrstr = sa2str(&sa, port, 0); /* cleanup the output */ - for (i = 0; i < strlen(addrstr); i++) { - if (iscntrl((unsigned char)addrstr[i]) || !isprint((unsigned char)addrstr[i])) - addrstr[i] = '.'; - } + if (addrstr) { + for (i = 0; i < strlen(addrstr); i++) { + if (iscntrl((unsigned char)addrstr[i]) || !isprint((unsigned char)addrstr[i])) + addrstr[i] = '.'; + } - chunk_appendf(&trash, " laddr=%s", addrstr); - free(addrstr); + chunk_appendf(&trash, " laddr=%s", addrstr); + free(addrstr); + } + else { + chunk_appendf(&trash, " laddr=?"); + } } salen = sizeof(sa); @@ -2323,12 +2328,17 @@ static int debug_iohandler_fd(struct appctx *appctx) port = 0; addrstr = sa2str(&sa, port, 0); /* cleanup the output */ - for (i = 0; i < strlen(addrstr); i++) { - if ((iscntrl((unsigned char)addrstr[i])) || !isprint((unsigned char)addrstr[i])) - addrstr[i] = '.'; + if (addrstr) { + for (i = 0; i < strlen(addrstr); i++) { + if ((iscntrl((unsigned char)addrstr[i])) || !isprint((unsigned char)addrstr[i])) + addrstr[i] = '.'; + } + chunk_appendf(&trash, " raddr=%s", addrstr); + free(addrstr); + } + else { + chunk_appendf(&trash, " raddr=?"); } - chunk_appendf(&trash, " raddr=%s", addrstr); - free(addrstr); } chunk_appendf(&trash, "\n"); -- 2.56.0
