From 6cb5690b3495741e9ece6f42ba4a85732932aa83 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Dag-Erling=20Sm=C3=B8rgrav?= <des@FreeBSD.org>
Date: Thu, 3 Aug 2023 15:13:45 +0000
Subject: [PATCH] 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 d09a3bf72c0b5f1779c52269671872368c99f02a)
(cherry picked from commit 92709431b14df6c0687446247ac57cfc189ee827)

Approved by:	so
---
 lib/libc/stdio/fflush.c  | 21 +++++++--------------
 lib/libc/stdio/fvwrite.c |  4 ++--
 lib/libc/stdio/wbuf.c    |  2 +-
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/lib/libc/stdio/fflush.c b/lib/libc/stdio/fflush.c
index 7ad6adedf53..61f23335106 100644
--- a/lib/libc/stdio/fflush.c
+++ b/lib/libc/stdio/fflush.c
@@ -103,8 +103,8 @@ __weak_reference(__fflush, fflush_unlocked);
 int
 __sflush(FILE *fp)
 {
-	unsigned char *p, *old_p;
-	int n, f, t, old_w;
+	unsigned char *p;
+	int n, f, t;
 
 	f = fp->_flags;
 	if ((f & __SWR) == 0)
@@ -119,26 +119,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 = 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 81e7ba89a64..26245acca1c 100644
--- a/lib/libc/stdio/fvwrite.c
+++ b/lib/libc/stdio/fvwrite.c
@@ -139,7 +139,7 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 				fp->_p += w;
 				old_p = fp->_p;
 				if (__fflush(fp) == EOF) {
-					if (old_p == fp->_p && errno == EINTR)
+					if (old_p == fp->_p)
 						fp->_p -= w;
 					goto err;
 				}
@@ -183,7 +183,7 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 				fp->_p += w;
 				old_p = fp->_p;
 				if (__fflush(fp) == EOF) {
-					if (old_p == fp->_p && errno == EINTR)
+					if (old_p == fp->_p)
 						fp->_p -= w;
 					goto err;
 				}
diff --git a/lib/libc/stdio/wbuf.c b/lib/libc/stdio/wbuf.c
index acbe379ad90..808cfa588cf 100644
--- a/lib/libc/stdio/wbuf.c
+++ b/lib/libc/stdio/wbuf.c
@@ -89,7 +89,7 @@ __swbuf(int c, FILE *fp)
 	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) {
+			if (fp->_p == old_p) {
 				fp->_p--;
 				fp->_w++;
 			}
-- 
2.45.0