Fix lots of bugs.

1. When fprintf(fp, "...%ls...", ...) encounters an encoding error,
do not destroy all the fp->_flags, which made the file permanently
unreadable and unwriteable.
2. Do not change fp->_flags at all in case of encoding errors.
Neither the manual nor POSIX ask for it, no other conversions set the
error indicator, and it isn't needed because the return value reports
failure and must be checked anyway.
3. Detect failure in mbrtowc(3), do not silently treat invalid bytes
in the format string as the end of the format string.
4. Detect failure of __find_arguments(), no matter whether due to
out of memory conditions or encoding errors, and gracefully fail
rather than accessing an invalid pointer.
5. Remove the pointless and slightly dangerous errno = EILSEQ overrides
after functions that already do that and are required by the standard
to do so.
OK jca@ on items 1, 2, and 5.
OK millert@ on the complete diff.
"Completely brutal mix of bugs."  deraadt@

1 files changed, 28 insertions, 18 deletions

diff --git a/lib/libc/stdio/vfprintf.c b/lib/libc/stdio/vfprintf.c
index aa971796342..3d7d41e103b 100644
--- a/lib/libc/stdio/vfprintf.c
+++ b/lib/libc/stdio/vfprintf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfprintf.c,v 1.70 2015/12/28 22:08:18 mmcc Exp $	*/
+/*	$OpenBSD: vfprintf.c,v 1.71 2016/01/04 15:47:47 schwarze Exp $	*/
 /*-
  * Copyright (c) 1990 The Regents of the University of California.
  * All rights reserved.
@@ -165,10 +165,8 @@ __wcsconv(wchar_t *wcsarg, int prec)
 		memset(&mbs, 0, sizeof(mbs));
 		p = wcsarg;
 		nbytes = wcsrtombs(NULL, (const wchar_t **)&p, 0, &mbs);
-		if (nbytes == (size_t)-1) {
-			errno = EILSEQ;
+		if (nbytes == (size_t)-1)
 			return (NULL);
-		}
 	} else {
 		/*
 		 * Optimisation: if the output precision is small enough,
@@ -188,10 +186,8 @@ __wcsconv(wchar_t *wcsarg, int prec)
 					break;
 				nbytes += clen;
 			}
-			if (clen == (size_t)-1) {
-				errno = EILSEQ;
+			if (clen == (size_t)-1)
 				return (NULL);
-			}
 		}
 	}
 	if ((convbuf = malloc(nbytes + 1)) == NULL)
@@ -203,7 +199,6 @@ __wcsconv(wchar_t *wcsarg, int prec)
 	if ((nbytes = wcsrtombs(convbuf, (const wchar_t **)&p,
 	    nbytes, &mbs)) == (size_t)-1) {
 		free(convbuf);
-		errno = EILSEQ;
 		return (NULL);
 	}
 	convbuf[nbytes] = '\0';
@@ -438,7 +433,11 @@ __vfprintf(FILE *fp, const char *fmt0, __va_list ap)
 		int hold = nextarg; \
 		if (argtable == NULL) { \
 			argtable = statargtable; \
-			__find_arguments(fmt0, orgap, &argtable, &argtablesiz); \
+			if (__find_arguments(fmt0, orgap, &argtable, \
+			    &argtablesiz) == -1) { \
+				ret = -1; \
+				goto error; \
+			} \
 		} \
 		nextarg = n2; \
 		val = GETARG(int); \
@@ -494,6 +493,10 @@ __vfprintf(FILE *fp, const char *fmt0, __va_list ap)
 				break;
 			}
 		}
+		if (n < 0) {
+			ret = -1;
+			goto error;
+		}
 		if (fmt != cp) {
 			ptrdiff_t m = fmt - cp;
 			if (m < 0 || m > INT_MAX - ret)
@@ -501,7 +504,7 @@ __vfprintf(FILE *fp, const char *fmt0, __va_list ap)
 			PRINT(cp, m);
 			ret += m;
 		}
-		if (n <= 0)
+		if (n == 0)
 			goto done;
 		fmt++;		/* skip over '%' */
 
@@ -564,8 +567,11 @@ reswitch:	switch (ch) {
 				nextarg = n;
 				if (argtable == NULL) {
 					argtable = statargtable;
-					__find_arguments(fmt0, orgap,
-					    &argtable, &argtablesiz);
+					if (__find_arguments(fmt0, orgap,
+					    &argtable, &argtablesiz) == -1) {
+						ret = -1;
+						goto error;
+					}
 				}
 				goto rflag;
 			}
@@ -590,8 +596,11 @@ reswitch:	switch (ch) {
 				nextarg = n;
 				if (argtable == NULL) {
 					argtable = statargtable;
-					__find_arguments(fmt0, orgap,
-					    &argtable, &argtablesiz);
+					if (__find_arguments(fmt0, orgap,
+					    &argtable, &argtablesiz) == -1) {
+						ret = -1;
+						goto error;
+					}
 				}
 				goto rflag;
 			}
@@ -640,8 +649,7 @@ reswitch:	switch (ch) {
 				mbseqlen = wcrtomb(buf,
 				    (wchar_t)GETARG(wint_t), &mbs);
 				if (mbseqlen == (size_t)-1) {
-					fp->_flags |= __SERR;
-					errno = EILSEQ;
+					ret = -1;
 					goto error;
 				}
 				cp = buf;
@@ -853,7 +861,7 @@ fp_common:
 				} else {
 					convbuf = __wcsconv(wcp, prec);
 					if (convbuf == NULL) {
-						fp->_flags = __SERR;
+						ret = -1;
 						goto error;
 					}
 					cp = convbuf;
@@ -1211,7 +1219,9 @@ __find_arguments(const char *fmt0, va_list ap, union arg **argtable,
 				break;
 			}
 		}
-		if (n <= 0)
+		if (n < 0)
+			return (-1);
+		if (n == 0)
 			goto done;
 		fmt++;		/* skip over '%' */