From e4162a8a6d81b1cd1cb30b92de3d3e21ce3158aa Mon Sep 17 00:00:00 2001 From: Jeff Moyer Date: Fri, 10 Nov 2023 16:57:07 -0500 Subject: [PATCH 1/3] FileUnopen: always return a valid file descriptor We have seen conserver crash due to a buffer overflow which was tracked down to the following code in Spawn(): if (pCLmall->fd != (CONSFILE *)0) { int fd; fd = FileUnopen(pCLmall->fd); pCLmall->fd = (CONSFILE *)0; CONDDEBUG((1, "Spawn(): closing Master() client fd %d", fd)); close(fd); * FD_CLR(fd, &rinit); FD_CLR(fd, &winit); } FileUnopen had returned -1 (which can happen for CONSFILEs of type SSLSocket), and that was passed to FD_CLR, which essentially uses it as an array index. The signature of the crash is as follows: *** buffer overflow detected ***: /usr/sbin/conserver terminated ======= Backtrace: ========= /lib64/libc.so.6(__fortify_fail+0x37)[0x7facde1987a7] /lib64/libc.so.6(+0x116922)[0x7facde196922] /lib64/libc.so.6(+0x118707)[0x7facde198707] /usr/sbin/conserver(+0x158d2)[0x558ddb5468d2] /usr/sbin/conserver(+0x2581a)[0x558ddb55681a] /usr/sbin/conserver(+0x1944f)[0x558ddb54a44f] /usr/sbin/conserver(+0x78f8)[0x558ddb5388f8] /lib64/libc.so.6(__libc_start_main+0xf5)[0x7facde0a2555] /usr/sbin/conserver(+0x7c79)[0x558ddb538c79] This happens after the server receives a HUP signal. There are only two callers of FileUnopen, and the above call site is the only one which uses the return value. For that reason, I decided to always return a valid file descriptor instead of changing the caller to check for -1. Note that FileUnopen() could still return -1 in theory: switch (cfp->ftype) { ... default: retval = -1; break; } However, after auditing the code, I don't see how we would have a CONSFILE that is not properly initialized with a type. If I missed such a case, then we would also need to modify the caller to check for -1. Signed-off-by: Jeff Moyer --- conserver/cutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conserver/cutil.c b/conserver/cutil.c index 342be53..49abc16 100644 --- a/conserver/cutil.c +++ b/conserver/cutil.c @@ -708,7 +708,7 @@ FileUnopen(CONSFILE *cfp) break; #if HAVE_OPENSSL case SSLSocket: - retval = -1; + retval = cfp->fd; break; #endif default: From ec846dfedde7931689ff0a56be5ba75a5aefc9ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= Date: Mon, 5 Feb 2024 21:16:51 +0100 Subject: [PATCH 2/3] fix SEGFAULT on early exit with IPv6 enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some command line options, like e.g -V, will cause conserver to exit before the IPv6 address variables are initialized. Avoid the calls to freeaddrinfo() in these cases. Signed-off-by: Bjørn Mork --- conserver/main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/conserver/main.c b/conserver/main.c index 8130d03..5035712 100644 --- a/conserver/main.c +++ b/conserver/main.c @@ -53,8 +53,8 @@ int fAll = 0, fNoinit = 0, fVersion = 0, fStrip = 0, fReopen = char *pcConfig = CONFIGFILE; int cMaxMemb = MAXMEMB; #if USE_IPV6 -struct addrinfo *bindAddr; -struct addrinfo *bindBaseAddr; +struct addrinfo *bindAddr = (struct addrinfo *)0; +struct addrinfo *bindBaseAddr = (struct addrinfo *)0; #else in_addr_t bindAddr = INADDR_ANY; unsigned short bindPort; @@ -781,8 +781,10 @@ DestroyDataStructures(void) #if USE_IPV6 /* clean up addrinfo stucts */ - freeaddrinfo(bindAddr); - freeaddrinfo(bindBaseAddr); + if ((struct addrinfo *)0 != bindAddr) + freeaddrinfo(bindAddr); + if ((struct addrinfo *)0 != bindBaseAddr) + freeaddrinfo(bindBaseAddr); #else if (myAddrs != (struct in_addr *)0) free(myAddrs); From 342fe1a4da3b9b9737e3ca97f8db22ebea22d057 Mon Sep 17 00:00:00 2001 From: Bryan Stansell Date: Sat, 17 Feb 2024 10:34:03 -0800 Subject: [PATCH 3/3] Try and find a valid image --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index e9e70ae..ce7bcb1 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -3,7 +3,7 @@ env: freebsd_13_task: freebsd_instance: - image_family: freebsd-13-0 + image_family: freebsd-13-2 install_script: - pkg install -y autoconf automake - ./package/setup-configure