NetBSD Problem Report #59218

From www@netbsd.org  Wed Mar 26 13:57:41 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits)
	 client-signature RSA-PSS (2048 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 77B401A9239
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 26 Mar 2025 13:57:41 +0000 (UTC)
Message-Id: <20250326135739.B4DB41A923D@mollari.NetBSD.org>
Date: Wed, 26 Mar 2025 13:57:39 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: blocklistd(8): possible fd leak and buffer overruns in bl_recv
X-Send-Pr-Version: www-1.0

>Number:         59218
>Category:       bin
>Synopsis:       blocklistd(8): possible fd leak and buffer overruns in bl_recv
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Mar 26 14:00:00 +0000 2025
>Last-Modified:  Sun Mar 30 01:55:00 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NulBSD Blocklistation
>Environment:
>Description:
In bl.c 1.4, the following logic appears:

   438		union {
   439			char ctrl[CMSG_SPACE(sizeof(int)) + CMSG_SPACE(CRED_SIZE)];
   440			uint32_t fd;
   441		} ua;
...
   446		union {
   447			bl_message_t bl;
   448			char buf[512];
   449		} ub;
...
   458		iov.iov_base = ub.buf;
   459		iov.iov_len = sizeof(ub);
...
   467		msg.msg_control = ua.ctrl;
   468		msg.msg_controllen = sizeof(ua.ctrl) + 100;
   469	
   470	        rlen = recvmsg(b->b_fd, &msg, 0);
...
   485			case SCM_RIGHTS:
   486				if (cmsg->cmsg_len != CMSG_LEN(sizeof(int))) {
   487					bl_log(b, LOG_ERR,
   488					    "%s: unexpected cmsg_len %d != %zu",
   489					    __func__, cmsg->cmsg_len,
   490					    CMSG_LEN(2 * sizeof(int)));
   491					continue;
   492				}
   493				memcpy(&bi->bi_fd, CMSG_DATA(cmsg), sizeof(bi->bi_fd));
   494				got |= GOT_FD;
   495				break;
...
   545			strlcpy(bi->bi_msg, ub.bl.bl_data, rem);

There are several issues here:

1. For reasons unclear to me, on line 468, this passes a control data buffer to the kernel that's 100 bytes longer than has been allocated on the stack, so the kernel will gladly overrun the buffer if the peer sends too much control data:

   467		msg.msg_control = ua.ctrl;
   468		msg.msg_controllen = sizeof(ua.ctrl) + 100;

   (Whisky tango foxtrot?)

2. Nothing prevents a peer from sending more than one file descriptor through the cmsg(3) API, but the logic on lines 485-491 just quietly ignores any excess file descriptors, leaking them.

3. Nothing in the ellipsis appears to NUL-terminate the buffer ub.bl.bl_data which is passed into strlcpy on line 545, but strlcpy absolutely requires its input to be NUL-terminated (one of the risks it introduces when naively replacing strncpy):

   545			strlcpy(bi->bi_msg, ub.bl.bl_data, rem);
>How-To-Repeat:
code inspection
>Fix:
1. Don't add 100 to msg_controllen.

2. Iterate over all the file descriptors in the cmsg record and close them.

3. Use strncpy instead of strlcpy (and maybe NUL-terminate the output if needed), or NUL-terminate the input.

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59218 CVS commit: src/external/bsd/blocklist/lib
Date: Wed, 26 Mar 2025 12:45:22 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Wed Mar 26 16:45:22 UTC 2025

 Modified Files:
 	src/external/bsd/blocklist/lib: bl.c

 Log Message:
 PR/59218: Taylor R Campbell: Remove extra slop on receive size and prevent
 fd leak by closing all passed descriptors if we receive more than one.


 To generate a diff of this commit:
 cvs rdiff -u -r1.6 -r1.7 src/external/bsd/blocklist/lib/bl.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59218 CVS commit: src/external/bsd/blocklist/lib
Date: Sat, 29 Mar 2025 21:53:59 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sun Mar 30 01:53:59 UTC 2025

 Modified Files:
 	src/external/bsd/blocklist/lib: bl.c

 Log Message:
 PR/59218: Taylor R Campbell: fix NUL termination


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/external/bsd/blocklist/lib/bl.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search