From nobody Wed Nov 08 00:53:00 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 4SQ6586X7Pz4ywB0; Wed, 8 Nov 2023 00:53:00 +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 4SQ65862vhz3S3B; Wed, 8 Nov 2023 00:53:00 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699404780; 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=Hrda63ZbMDLXv8oaH/qU2D+xb9AesFJFyIo8WH9wtu0=; b=jEsVO8kgQ+gZSborcbOFRDTEu6NjbMlHnUF5l7m/966bPQbjNnzUVN0hQZgRoENrAZyNUi DW2pwmRJnaCwfhEmXpzp7pzTN7JMppAZDhEw5AsOf/zUR1r8v1z6+xNqGJYP4fJJx7TXf1 KP9EHe8OObIg+rutBBMsKD1idqwgThE1CZzKuobSyBjxWuKe5HJU/wyUDOuS+wWDf9hKNZ WNY18YQkw58HwTms5qVvN6A0+eqJ9GnCq1RkWKhXYjarzL0i2TabLIpt9DWdsXI3CY7D13 NsiEeJPkZ4Y/ewG4Pp3J3BVs68LK4PnISBSi1MUe4CzaoehJNBmiod4rPZXVCA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699404780; 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=Hrda63ZbMDLXv8oaH/qU2D+xb9AesFJFyIo8WH9wtu0=; b=Oc/c+ERHXzs7HZo6wLUAamuqooJMlk1roIpoRYaVPwKmaZ5xtge36LGO1t3dyM/0cInOk5 cT+RsKH1xq7XmlSbjpPHd0T8t0uqe57Hn8iubIK34jtRy6tMAVB22mv9biRV3k2LuTStbG dAgKbTIFZQV1edS4oRk298UzYvNtZCqz1naZZHbeGTXbUbbcHKvbY+sOs+nx78J51ksrhM d2ybJhEgi5/JXGUJoZv6r6p6vsAuB3s4m09gcw3vAkMPcTHc1c/uGs5i3pc57JGV923gqI SC0KaK+P0yWplb24PSZc+VWvA9ViYffmDesCp8MlT1EBGIeUcr1TPcJOGIMdsA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1699404780; a=rsa-sha256; cv=none; b=PLSQKYZ25TtOte0FDyq1EV9I9UdUDjtAkwW/iBFkhmM8cODpsPZlEXveHlu0lHMT/rARvN kbOxvpFJIhBH6ybhlROYHv5VygjHPgHeSvwoLWaVz9/+F2qdrgf9bIRQFof/U8KkG3bfkk yN7ph4b/D2mMF1Qm1Of7aHKxFPusY9V1yQL5I4fLsWyHTjWZ3Qt+vQeoVY2c2XSuPypH8f L30lmKLhETmwNKMIDJcF/5bMhPDOhP6pT+1ah6yW5HlssdUD9RTyvXe7hz1IWpDNM4rKBd 0580ZaMKXqsyagvgg2d8VFhF7noRLx9S+mdzO70ZoF7pHgTviRO0zPVqga7ShQ== 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 4SQ658563Lz1C5c; Wed, 8 Nov 2023 00:53:00 +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 3A80r0b9087348; Wed, 8 Nov 2023 00:53:00 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3A80r0wu087345; Wed, 8 Nov 2023 00:53:00 GMT (envelope-from git) Date: Wed, 8 Nov 2023 00:53:00 GMT Message-Id: <202311080053.3A80r0wu087345@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Ed Maste Subject: git: d51a39b13ee4 - releng/13.2 - fflush: correct buffer handling in __sflush 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: emaste X-Git-Repository: src X-Git-Refname: refs/heads/releng/13.2 X-Git-Reftype: branch X-Git-Commit: d51a39b13ee4de5410f6c01a357eafc6248f6724 Auto-Submitted: auto-generated The branch releng/13.2 has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=d51a39b13ee4de5410f6c01a357eafc6248f6724 commit d51a39b13ee4de5410f6c01a357eafc6248f6724 Author: Dag-Erling Smørgrav AuthorDate: 2023-08-03 15:08:03 +0000 Commit: Ed Maste CommitDate: 2023-11-08 00:48:03 +0000 fflush: correct buffer handling in __sflush This fixes CVE-2014-8611 correctly. The commit that purported to fix CVE-2014-8611 (805288c2f062) only hid it behind another bug. Two later commits, 86a16ada1ea6 and 44cf1e5eb470, attempted to address this new bug but mostly just confused the issue. This commit rolls back the three previous changes and fixes CVE-2014-8611 correctly. The key to understanding the bug (and the fix) is that `_w` has different meanings for different stream modes. If the stream is unbuffered, it is always zero. If the stream is fully buffered, it is the amount of space remaining in the buffer (equal to the buffer size when the buffer is empty and zero when the buffer is full). If the stream is line-buffered, it is a negative number reflecting the amount of data in the buffer (zero when the buffer is empty and negative buffer size when the buffer is full). At the heart of `fflush()`, we call the stream's write function in a loop, where `t` represents the return value from the last call and `n` the amount of data that remains to be written. When the write function fails, we need to move the unwritten data to the top of the buffer (unless nothing was written) and adjust `_p` (which points to the next free location in the buffer) and `_w` accordingly. These variables have already been set to the values they should have after a successful flush, so instead of adjusting them down to reflect what was written, we're adjusting them up to reflect what remains. The bug was that while `_p` was always adjusted, we only adjusted `_w` if the stream was fully buffered. The fix is to also adjust `_w` for line-buffered streams. Everything else is just noise. Fixes: 805288c2f062 Fixes: 86a16ada1ea6 Fixes: 44cf1e5eb470 Sponsored by: Klara, Inc. (cherry picked from commit 1f90b4edffe815aebb35e74b79e10593b31f6b75) (cherry picked from commit 1e99535be2ea9c0ef8bc57fc885e9c01fa95d2dd) (cherry picked from commit ccdd8337f9cbd7d34e2e95df1440dd5f7225d0b4) (cherry picked from commit d09a3bf72c0b5f1779c52269671872368c99f02a) (cherry picked from commit 92709431b14df6c0687446247ac57cfc189ee827) (cherry picked from commit 6cb5690b3495741e9ece6f42ba4a85732932aa83) (cherry picked from commit 418f026bd5a5084c1c4e2e91ad38051f6caa928c) (cherry picked from commit abe12d2f4ce31c3da0961b1b0a58df11f5a41e19) (cherry picked from commit 59ec3ffdd7ce85f32ea833e8024f7bacd36d4e97) (cherry picked from commit 4e0e01bf6511c28212d7dff94fe131a502e13026) (cherry picked from commit d2c65a1c948648f11342274029a3f18b90aa58d2) (cherry picked from commit 0b7939d725ba0ca903c5f8a3ca6d74347eb88690) Approved by: so Approved by: re (implicit) Security: SA-23:15.stdio Sponsored by: The FreeBSD Foundation --- lib/libc/stdio/fflush.c | 27 ++++++++++----------------- lib/libc/stdio/fvwrite.c | 14 ++------------ lib/libc/stdio/wbuf.c | 12 ++---------- 3 files changed, 14 insertions(+), 39 deletions(-) diff --git a/lib/libc/stdio/fflush.c b/lib/libc/stdio/fflush.c index f59565abd369..75f145fae6a3 100644 --- a/lib/libc/stdio/fflush.c +++ b/lib/libc/stdio/fflush.c @@ -105,11 +105,11 @@ __weak_reference(__fflush, fflush_unlocked); int __sflush(FILE *fp) { - unsigned char *p, *old_p; - int n, t, old_w; + unsigned char *p; + int n, f, t; - t = fp->_flags; - if ((t & __SWR) == 0) + f = fp->_flags; + if ((f & __SWR) == 0) return (0); if ((p = fp->_bf._base) == NULL) @@ -121,26 +121,19 @@ __sflush(FILE *fp) * Set these immediately to avoid problems with longjmp and to allow * exchange buffering (via setvbuf) in user write function. */ - old_p = fp->_p; fp->_p = p; - old_w = fp->_w; - fp->_w = t & (__SLBF|__SNBF) ? 0 : fp->_bf._size; + fp->_w = f & (__SLBF|__SNBF) ? 0 : fp->_bf._size; for (; n > 0; n -= t, p += t) { t = _swrite(fp, (char *)p, n); if (t <= 0) { - /* Reset _p and _w. */ - if (p > fp->_p) { + if (p > fp->_p) /* Some was written. */ memmove(fp->_p, p, n); - fp->_p += n; - if ((fp->_flags & (__SLBF | __SNBF)) == 0) - fp->_w -= n; - /* conditional to handle setvbuf */ - } else if (p == fp->_p && errno == EINTR) { - fp->_p = old_p; - fp->_w = old_w; - } + /* Reset _p and _w. */ + fp->_p += n; + if ((fp->_flags & __SNBF) == 0) + fp->_w -= n; fp->_flags |= __SERR; return (EOF); } diff --git a/lib/libc/stdio/fvwrite.c b/lib/libc/stdio/fvwrite.c index b1b363e6f80d..50b32b8eca6e 100644 --- a/lib/libc/stdio/fvwrite.c +++ b/lib/libc/stdio/fvwrite.c @@ -38,7 +38,6 @@ static char sccsid[] = "@(#)fvwrite.c 8.1 (Berkeley) 6/4/93"; #include __FBSDID("$FreeBSD$"); -#include #include #include #include @@ -55,7 +54,6 @@ int __sfvwrite(FILE *fp, struct __suio *uio) { size_t len; - unsigned char *old_p; char *p; struct __siov *iov; int w, s; @@ -139,12 +137,8 @@ __sfvwrite(FILE *fp, struct __suio *uio) COPY(w); /* fp->_w -= w; */ /* unneeded */ fp->_p += w; - old_p = fp->_p; - if (__fflush(fp) == EOF) { - if (old_p == fp->_p && errno == EINTR) - fp->_p -= w; + if (__fflush(fp)) goto err; - } } else if (len >= (w = fp->_bf._size)) { /* write directly */ w = _swrite(fp, p, w); @@ -183,12 +177,8 @@ __sfvwrite(FILE *fp, struct __suio *uio) COPY(w); /* fp->_w -= w; */ fp->_p += w; - old_p = fp->_p; - if (__fflush(fp) == EOF) { - if (old_p == fp->_p && errno == EINTR) - fp->_p -= w; + if (__fflush(fp)) goto err; - } } else if (s >= (w = fp->_bf._size)) { w = _swrite(fp, p, w); if (w <= 0) diff --git a/lib/libc/stdio/wbuf.c b/lib/libc/stdio/wbuf.c index 666bbf87aadd..deb111162741 100644 --- a/lib/libc/stdio/wbuf.c +++ b/lib/libc/stdio/wbuf.c @@ -52,7 +52,6 @@ __FBSDID("$FreeBSD$"); int __swbuf(int c, FILE *fp) { - unsigned char *old_p; int n; /* @@ -88,15 +87,8 @@ __swbuf(int c, FILE *fp) } fp->_w--; *fp->_p++ = c; - old_p = fp->_p; - if (++n == fp->_bf._size || (fp->_flags & __SLBF && c == '\n')) { - if (__fflush(fp) != 0) { - if (fp->_p == old_p && errno == EINTR) { - fp->_p--; - fp->_w++; - } + if (++n == fp->_bf._size || (fp->_flags & __SLBF && c == '\n')) + if (__fflush(fp) != 0) return (EOF); - } - } return (c); }