From nobody Tue Dec 16 16:13:20 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4dW2686sBmz6LBXJ for ; Tue, 16 Dec 2025 16:13:20 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4dW2685s3vz3G8t for ; Tue, 16 Dec 2025 16:13:20 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1765901600; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+LDE2++fwsYLoz1zjvmAyxI/S3XmTMVSa7hlyRGGtsA=; b=VKk9vkwP+jbm2o+H8GFwY8RJztaXFAHnncougF0TehN7O+oL72+zf05kxMbtJ6npWAKL+a VUljZ9tQ28gVq0riMzhLaqq+1zf4ffc7SbZAALQ8uRhmirgrbNJI3HNVTp2zj9zwpMLl7v Jgt2JUTxm0RzLOAiwE7oL5mV8/33dfpk6wVtTe0eGu4JpMkJl+7l52qX23KJWGmlUqs+o7 gXGL1LclKyPQ3WcdvAuE2bNWIo+TusuSAvbPvvlEKq+FPU1VTk+S2z1iXLQN1VHMXrpsUG /kSiRlVeY9B7giU2y1VOkyoLJwB9W1L3UFQl3RRk67lZtOhkaIAaIMTYbiqZPA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1765901600; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+LDE2++fwsYLoz1zjvmAyxI/S3XmTMVSa7hlyRGGtsA=; b=XBJFI3B9db4CiAh8dqVZDMDQBHkM5LHu1yxaB1flD61A2CLbojjBf7JFpfp8iW8/bc82WT licpb88z4IvMEqIjlUv58ybCnywOhcUq+Tk8FxrjsPblIlt1vEtCdAprCj0JpVwvpqxiRu xhkkYegvjHTtpr0mVNPLB/5NBIgAwk/2TTey9AT0SzSBGXDI054iluQI0ZHeB1sdu1XBNz XtmcRBl+eo40TSJg9nUL3wT9xsZYOVD4El5y35hH8IG421pzqe1fnEVL3+EsDiub3lg5Z6 FQzGwPNUIuo46pfppvn/fy3bZwhgZ4zz5J9IIQsF2DWx+KGJ6i86OwdG9lTHeQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1765901600; a=rsa-sha256; cv=none; b=QCPJQj1pZTMyWpX8MsAGFqvM+d9MfO4mDNJ5DI6rRwWiZiK7Dp6sTLn7UX18FzGV83BhR6 Yphywrcv2tUJm83zYYdtcwpglwIqHlwS3pvr24nArXX/eVPRD8ScElxBWPPlhRmH8VaNGY BczyIGrO1c6BHmsQQdre/Pqh5IeqxCXkk3iq8m1nMwMIpfRVcVZngt4mYChxtDsMYc7XJA /jeHjhwKqgpFzHMwNEJgNUH1fD5fl8NGX6n1B2DUUNZ2gmA9xr4KpjVxPpmGYcCTa1A1hR AOaIQnKJ6tOKee9SNeRssHIs47FAol2GBSqeVFA0F75mhzAZP1S4mhRm8fvtsQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4dW26858CBz1CRd for ; Tue, 16 Dec 2025 16:13:20 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 441f3 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Tue, 16 Dec 2025 16:13:20 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav Subject: git: a34c50fbd2a5 - main - ipfilter: Prevent stack buffer overflow List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: des X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: a34c50fbd2a52bb63acde82e5aec4cb57880e39b Auto-Submitted: auto-generated Date: Tue, 16 Dec 2025 16:13:20 +0000 Message-Id: <69418520.441f3.317f300b@gitrepo.freebsd.org> The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=a34c50fbd2a52bb63acde82e5aec4cb57880e39b commit a34c50fbd2a52bb63acde82e5aec4cb57880e39b Author: Dag-Erling Smørgrav AuthorDate: 2025-12-16 16:11:24 +0000 Commit: Dag-Erling Smørgrav CommitDate: 2025-12-16 16:12:36 +0000 ipfilter: Prevent stack buffer overflow When copying ipfs data from user space, don't just check that the payload length is nonzero, but also that it does not exceed the size of the stack buffer we're copying it into. While we're at it, use a union to create a buffer of the exact size we need instead of guessing that 2048 will be enough (and not too much). Finally, check the size of the payload once it gets to where it's used. MFC after: 3 days Reported by: Ilja Van Sprundel Reviewed by: cy Differential Revision: https://reviews.freebsd.org/D54194 --- sbin/ipf/libipf/interror.c | 5 ++++ sys/netpfil/ipfilter/netinet/ip_sync.c | 51 ++++++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/sbin/ipf/libipf/interror.c b/sbin/ipf/libipf/interror.c index 2bbecaa154e6..a7e6f4a5e431 100644 --- a/sbin/ipf/libipf/interror.c +++ b/sbin/ipf/libipf/interror.c @@ -483,6 +483,11 @@ log" }, { 110019, "sync update could not find NAT entry" }, { 110020, "unrecognised sync NAT command" }, { 110021, "ioctls are not handled with sync" }, + /* missing entries 110022-110024 */ + { 110025, "invalid payload length (sync create state)" }, + { 110026, "invalid payload length (sync update state)" }, + { 110027, "invalid payload length (sync create NAT)" }, + { 110028, "invalid payload length (sync update NAT)" }, /* -------------------------------------------------------------------------- */ { 120001, "null data pointer for iterator" }, { 120002, "unit outside of acceptable range" }, diff --git a/sys/netpfil/ipfilter/netinet/ip_sync.c b/sys/netpfil/ipfilter/netinet/ip_sync.c index f6bc7e7fbe2a..b0be68148a18 100644 --- a/sys/netpfil/ipfilter/netinet/ip_sync.c +++ b/sys/netpfil/ipfilter/netinet/ip_sync.c @@ -409,13 +409,16 @@ ipf_sync_write(ipf_main_softc_t *softc, struct uio *uio) { ipf_sync_softc_t *softs = softc->ipf_sync_soft; synchdr_t sh; - - /* - * THIS MUST BE SUFFICIENT LARGE TO STORE - * ANY POSSIBLE DATA TYPE - */ - char data[2048]; - + union ipf_sync_data { + union ipf_sync_state_data { + ipstate_t create; + synctcp_update_t update; + } state; + union ipf_sync_nat_data { + nat_t create; + syncupdent_t update; + } nat; + } data; int err = 0; # if defined(__NetBSD__) || defined(__FreeBSD__) @@ -494,18 +497,18 @@ ipf_sync_write(ipf_main_softc_t *softc, struct uio *uio) * needed for the request */ - /* not supported */ - if (sh.sm_len == 0) { + /* too short or too long */ + if (sh.sm_len == 0 || sh.sm_len > sizeof(data)) { if (softs->ipf_sync_debug > 2) - printf("uiomove(data zero length %s\n", - "not supported"); + printf("uiomove(data) invalid length %d\n", + sh.sm_len); IPFERROR(110006); return (EINVAL); } if (uio->uio_resid >= sh.sm_len) { - err = UIOMOVE(data, sh.sm_len, UIO_WRITE, uio); + err = UIOMOVE(&data, sh.sm_len, UIO_WRITE, uio); if (err) { if (softs->ipf_sync_debug > 2) @@ -519,9 +522,9 @@ ipf_sync_write(ipf_main_softc_t *softc, struct uio *uio) sh.sm_len); if (sh.sm_table == SMC_STATE) - err = ipf_sync_state(softc, &sh, data); + err = ipf_sync_state(softc, &sh, &data); else if (sh.sm_table == SMC_NAT) - err = ipf_sync_nat(softc, &sh, data); + err = ipf_sync_nat(softc, &sh, &data); if (softs->ipf_sync_debug > 7) printf("[%d] Finished with error %d\n", sh.sm_num, err); @@ -651,6 +654,11 @@ ipf_sync_state(ipf_main_softc_t *softc, synchdr_t *sp, void *data) { case SMC_CREATE : + if (sp->sm_len != sizeof(sn)) { + IPFERROR(110025); + err = EINVAL; + break; + } bcopy(data, &sn, sizeof(sn)); KMALLOC(is, ipstate_t *); if (is == NULL) { @@ -717,6 +725,11 @@ ipf_sync_state(ipf_main_softc_t *softc, synchdr_t *sp, void *data) break; case SMC_UPDATE : + if (sp->sm_len != sizeof(su)) { + IPFERROR(110026); + err = EINVAL; + break; + } bcopy(data, &su, sizeof(su)); if (softs->ipf_sync_debug > 4) @@ -892,6 +905,11 @@ ipf_sync_nat(ipf_main_softc_t *softc, synchdr_t *sp, void *data) break; } + if (sp->sm_len != sizeof(*nat)) { + IPFERROR(110027); + err = EINVAL; + break; + } nat = (nat_t *)data; bzero((char *)n, offsetof(nat_t, nat_age)); bcopy((char *)&nat->nat_age, (char *)&n->nat_age, @@ -915,6 +933,11 @@ ipf_sync_nat(ipf_main_softc_t *softc, synchdr_t *sp, void *data) break; case SMC_UPDATE : + if (sp->sm_len != sizeof(su)) { + IPFERROR(110028); + err = EINVAL; + break; + } bcopy(data, &su, sizeof(su)); for (sl = softs->syncnattab[hv]; (sl != NULL);