From nobody Tue Aug 08 17:02:06 2023 X-Original-To: dev-commits-src-all@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 4RKzxp2zYhz4pq77; Tue, 8 Aug 2023 17:02:06 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RKzxp2Ylzz4K8f; Tue, 8 Aug 2023 17:02:06 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1691514126; 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=kScV6cQpexuqzYeWBBWszbHOpX9bCOmclkPTpGocx9Q=; b=GTtTDg2q7SiaQR+VOzjt7UyqRBa3dd85EE/sVHbNqCrSxK6gyJqLuMhwLcohQO9J+XpbI0 9bpWngetuHRN+yacJs7I0nGYQaNWEPyQXsBpDkQX7C6QUsA98naRVlW7p7noWDsKm89hcE peltppTnEoI1UrBhuqYJf0QsZpNvfhU758v1Undxgk97j+W52fmfp2iTSYGu0PkwloyYyZ HvqGwtOArJwQaa8O9OETXkLTh4ch0F3s7yxQU51UNoYhFXeVbm4x6S+VKla9RYxVID0Na2 T35PqnUFH/XtYDPQJoUQjywyZ38UwSbof8P9A4yygUGPxSy3R2MBvlq7G7PFbA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1691514126; 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=kScV6cQpexuqzYeWBBWszbHOpX9bCOmclkPTpGocx9Q=; b=ePwqQZrdydLuOmqLrC1kghHQfrOPWCVmsRoci1OnAlKW+AsBhyZjBvzIesB6Ldbi3evVlm 5//9g7GKaFa7RA1DEu4jYEx309mUWuny+g1oDwGwC7shEc9staM08IWGvGaFGuUeMcQYqM jzdNmZju0V4PQMHm6sxTXSprVF2a9rfSvbpIHxAW9y6WSB1O9msZ5DI+n7JCxXBK9KLTlx e9mbO226FBv+av90eK8bolsQPXuDEjd5JEb9mEUwXkW3yviVz89vXtMEg4+iv4H/bKBSTO zgX5QbHFNXqFwjTTqLXtV1iwDwv5xHuahl/AOEGxRXXxjG5KhPwFaw73i/i1eA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1691514126; a=rsa-sha256; cv=none; b=Yg2+Xa9yhcaZTyoclXdtQhkVH5ckEEbH868yW35pWnBcKlDkNIwbDl4zwc68phWwLaqaKt o+VfdaPPJu5vIq9Q9VohYm3d2WPrdyglwt1Dw7ehCgEuHDrX7F5Y6hnK/ImlpMAKrluJVt Hyb/EI90uEcZ0iS565NIFOROGemeMKZZViKzEnKYP7fW7e4w1p87s6twk7GJqL3T8DDJMV 6AneXDcQzx9wZ7d/UDJehi1flfO8DSoQFDB24rlWVTo/KiHf7Slifw4mizmILr2sGf2sdK 6xcAx0zknnar2g+E2MhTddmKP1ssqDTtym7OtDY+ulylW3kzdyW+8fIAGmE2hw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4RKzxp1XtCzhdl; Tue, 8 Aug 2023 17:02:06 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 378H264a090397; Tue, 8 Aug 2023 17:02:06 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 378H26N9090396; Tue, 8 Aug 2023 17:02:06 GMT (envelope-from git) Date: Tue, 8 Aug 2023 17:02:06 GMT Message-Id: <202308081702.378H26N9090396@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kyle Evans Subject: git: 2f489a509e61 - main - libc: fix some overflow scenarios in vis(3) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 2f489a509e615c46be6f7c6aa7cea161f50f18af Auto-Submitted: auto-generated The branch main has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=2f489a509e615c46be6f7c6aa7cea161f50f18af commit 2f489a509e615c46be6f7c6aa7cea161f50f18af Author: Kyle Evans AuthorDate: 2023-08-08 17:01:28 +0000 Commit: Kyle Evans CommitDate: 2023-08-08 17:01:52 +0000 libc: fix some overflow scenarios in vis(3) The previous incarnation of this would call wcrtomb() on the destination buffer, and only check for overflow *after* it's happened. Additionally, the conversion error / VIS_NOLOCALE path also didn't check for overflow, and the overflow check at the end didn't account for the fact that we still need to write a NUL terminator afterward. Start by only doing the multibyte conversion into mbdst directly if we have enough buffer space to guarantee it'll fit. An additional MB_CUR_MAX buffer has been stashed on the stack to write into if we're cutting it close at the end of the buffer, since we don't really have a good way to determine the length of the wchar_t without just doing the conversion. We'll do the conversion into the buffer that's guaranteed to fit, then copy it over if the copy won't overflow. The byte-for-byte overflow is a little bit easier, as we simply check for overflow with each byte written and make sure we can still NUL terminate after. Tests added to exercise these edge cases. Reviewed by: des Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D41328 --- contrib/libc-vis/vis.c | 54 +++++++++++++++++++++---- contrib/netbsd-tests/lib/libc/gen/t_vis.c | 66 +++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 8 deletions(-) diff --git a/contrib/libc-vis/vis.c b/contrib/libc-vis/vis.c index c43186a44b51..fe3939448087 100644 --- a/contrib/libc-vis/vis.c +++ b/contrib/libc-vis/vis.c @@ -395,14 +395,16 @@ static int istrsenvisx(char **mbdstp, size_t *dlen, const char *mbsrc, size_t mblength, int flags, const char *mbextra, int *cerr_ptr) { + char mbbuf[MB_CUR_MAX]; wchar_t *dst, *src, *pdst, *psrc, *start, *extra; size_t len, olen; uint64_t bmsk, wmsk; wint_t c; visfun_t f; int clen = 0, cerr, error = -1, i, shft; - char *mbdst, *mdst; - ssize_t mbslength, maxolen; + char *mbdst, *mbwrite, *mdst; + ssize_t mbslength; + size_t maxolen; mbstate_t mbstate; _DIAGASSERT(mbdstp != NULL); @@ -541,8 +543,33 @@ istrsenvisx(char **mbdstp, size_t *dlen, const char *mbsrc, size_t mblength, olen = 0; bzero(&mbstate, sizeof(mbstate)); for (dst = start; len > 0; len--) { - if (!cerr) - clen = wcrtomb(mbdst, *dst, &mbstate); + if (!cerr) { + /* + * If we have at least MB_CUR_MAX bytes in the buffer, + * we'll just do the conversion in-place into mbdst. We + * need to be a little more conservative when we get to + * the end of the buffer, as we may not have MB_CUR_MAX + * bytes but we may not need it. + */ + if (maxolen - olen > MB_CUR_MAX) + mbwrite = mbdst; + else + mbwrite = mbbuf; + clen = wcrtomb(mbwrite, *dst, &mbstate); + if (clen > 0 && mbwrite != mbdst) { + /* + * Don't break past our output limit, noting + * that maxolen includes the nul terminator so + * we can't write past maxolen - 1 here. + */ + if (olen + clen >= maxolen) { + errno = ENOSPC; + goto out; + } + + memcpy(mbdst, mbwrite, clen); + } + } if (cerr || clen < 0) { /* * Conversion error, process as a byte(s) instead. @@ -557,16 +584,27 @@ istrsenvisx(char **mbdstp, size_t *dlen, const char *mbsrc, size_t mblength, shft = i * NBBY; bmsk = (uint64_t)0xffLL << shft; wmsk |= bmsk; - if ((*dst & wmsk) || i == 0) + if ((*dst & wmsk) || i == 0) { + if (olen + clen + 1 >= maxolen) { + errno = ENOSPC; + goto out; + } + mbdst[clen++] = (char)( (uint64_t)(*dst & bmsk) >> shft); + } } cerr = 1; } - /* If this character would exceed our output limit, stop. */ - if (olen + clen > (size_t)maxolen) - break; + + /* + * We'll be dereferencing mbdst[clen] after this to write the + * nul terminator; the above paths should have checked for a + * possible overflow already. + */ + assert(olen + clen < maxolen); + /* Advance output pointer by number of bytes written. */ mbdst += clen; /* Advance buffer character pointer. */ diff --git a/contrib/netbsd-tests/lib/libc/gen/t_vis.c b/contrib/netbsd-tests/lib/libc/gen/t_vis.c index adb0930a300a..80800bf8b31f 100644 --- a/contrib/netbsd-tests/lib/libc/gen/t_vis.c +++ b/contrib/netbsd-tests/lib/libc/gen/t_vis.c @@ -175,6 +175,68 @@ ATF_TC_BODY(strvis_locale, tc) } #endif /* VIS_NOLOCALE */ +#ifdef __FreeBSD__ +#define STRVIS_OVERFLOW_MARKER 0xff /* Arbitrary */ + +ATF_TC(strvis_overflow_mb); +ATF_TC_HEAD(strvis_overflow_mb, tc) +{ + atf_tc_set_md_var(tc, "descr", "Test strvis(3) multi-byte overflow"); +} + +ATF_TC_BODY(strvis_overflow_mb, tc) +{ + const char src[] = "\xf0\x9f\xa5\x91"; + /* Extra byte to detect overflow */ + char dst[sizeof(src) + 1]; + int n; + + setlocale(LC_CTYPE, "en_US.UTF-8"); + + /* Arbitrary */ + memset(dst, STRVIS_OVERFLOW_MARKER, sizeof(dst)); + + /* + * If we only provide four bytes of buffer, we shouldn't be able encode + * a full 4-byte sequence. + */ + n = strnvis(dst, 4, src, VIS_SAFE); + ATF_REQUIRE(dst[4] == STRVIS_OVERFLOW_MARKER); + ATF_REQUIRE(n == -1); + + n = strnvis(dst, sizeof(src), src, VIS_SAFE); + ATF_REQUIRE(n == sizeof(src) - 1); +} + +ATF_TC(strvis_overflow_c); +ATF_TC_HEAD(strvis_overflow_c, tc) +{ + atf_tc_set_md_var(tc, "descr", "Test strvis(3) C locale overflow"); +} + +ATF_TC_BODY(strvis_overflow_c, tc) +{ + const char src[] = "AAAA"; + /* Extra byte to detect overflow */ + char dst[sizeof(src) + 1]; + int n; + + /* Arbitrary */ + memset(dst, STRVIS_OVERFLOW_MARKER, sizeof(dst)); + + /* + * If we only provide four bytes of buffer, we shouldn't be able encode + * 4 bytes of input. + */ + n = strnvis(dst, 4, src, VIS_SAFE | VIS_NOLOCALE); + ATF_REQUIRE(dst[4] == STRVIS_OVERFLOW_MARKER); + ATF_REQUIRE(n == -1); + + n = strnvis(dst, sizeof(src), src, VIS_SAFE | VIS_NOLOCALE); + ATF_REQUIRE(n == sizeof(src) - 1); +} +#endif /* __FreeBSD__ */ + ATF_TP_ADD_TCS(tp) { @@ -185,6 +247,10 @@ ATF_TP_ADD_TCS(tp) #ifdef VIS_NOLOCALE ATF_TP_ADD_TC(tp, strvis_locale); #endif /* VIS_NOLOCALE */ +#ifdef __FreeBSD__ + ATF_TP_ADD_TC(tp, strvis_overflow_mb); + ATF_TP_ADD_TC(tp, strvis_overflow_c); +#endif return atf_no_error(); }