fprintf() crashes on wide-oriented stream.

[Takashi Yano <takashi.yano@nifty.ne.jp>]

[-- Attachment #1: Type: text/plain, Size: 1522 bytes --]

Hi,

I noticed that the following test case crashes at printf() with current
newlib.


#include <stdio.h>
#include <wchar.h>
#include <locale.h>

int main()
{
	setlocale(LC_ALL, "C.UTF-8");
	wprintf(L"%ls\n", L"aaaa"); /* or fwide(stdout, 1); */
	printf("%ls\n", L"bbbb");  /* <--- crash here */
	return 0;
}


I looked into this problem and found the cause.

A narrow char string which can be odd bytes in length is cast into
a wide char string which should be even bytes in length in __sprint_r/
__sfputs_r based on the __SWID flag. As a result, if the length is
odd bytes, the reading buffer runs over the buffer length, which causes
a crash. If the length is even bytes, crash does not happen, but garbage
is printed. This hapens if printf("%ls\r\n", L"bbbb"); is used instead.
                                      ^^

The same issue seemed to be reported ten years ago.
https://sourceware.org/pipermail/newlib/2013/010831.html

I have built a patch attached for this issue.

With this patch, __sfputs_r/__sprint_r is split into two versions, one
is for vfprintf which does not handle wide string, and the other (newly
introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
string. Please note that fprintf gets working for wide orient stream
just like BSD libc, which behaves differently from GNU libc.

This patch also fixes nano-vfprintf.c as well as vfprintf.c/vfwprintf.c
in the same manner.

Could someone please review the patch?
Thanks in advance.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 0001-newlib-libc-Fix-crash-on-fprintf-to-a-wide-oriented-.patch --]
[-- Type: text/plain, Size: 7549 bytes --]

From 1bafca861ed191444f16dedb4b1987404ee7830e Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Tue, 26 Sep 2023 08:19:07 +0900
Subject: [PATCH] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

Previously, fprintf() on a wide-oriented stream crashes or outputs
garbage. This is because a narrow char string which can be odd bytes
in length is cast into a wide char string which should be even
bytes in length in __sprint_r/__sfputs_r based on the __SWID flag.
As a result, if the length is odd bytes, the reading buffer runs over
the buffer length, which causes a crash. If the length is even bytes,
garbage is printed.

With this patch, __sfputs_r/__sprint_r is split into two versions;
one is for vfprintf which does not handle wide string, and the other
(newly introduced __sfputws_r/__swprin_r) is for vfwprintf which
handles wide string. Please note that fprintf() gets working for
wide-oriented stream just like BSD libc, which behaves differently
from GNU libc.

Revewed-by:
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 newlib/libc/stdio/nano-vfprintf.c | 51 ++-----------------
 newlib/libc/stdio/vfprintf.c      | 44 ++---------------
 newlib/libc/stdio/vfwprintf.c     | 81 ++++++++++++++++++++++++++++++-
 3 files changed, 89 insertions(+), 87 deletions(-)

diff --git a/newlib/libc/stdio/nano-vfprintf.c b/newlib/libc/stdio/nano-vfprintf.c
index 0d42a940f..d2143ef08 100644
--- a/newlib/libc/stdio/nano-vfprintf.c
+++ b/newlib/libc/stdio/nano-vfprintf.c
@@ -356,32 +356,7 @@ __sprint_r (struct _reent *ptr,
       uio->uio_iovcnt = 0;
       return 0;
     }
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-    if (fp->_flags2 & __SWID)
-      {
-	struct __siov *iov;
-	wchar_t *p;
-	int i, len;
-
-	iov = uio->uio_iov;
-	for (; uio->uio_resid != 0;
-	     uio->uio_resid -= len * sizeof (wchar_t), iov++)
-	  {
-	    p = (wchar_t *) iov->iov_base;
-	    len = iov->iov_len / sizeof (wchar_t);
-	    for (i = 0; i < len; i++)
-	      {
-		if (_fputwc_r (ptr, p[i], fp) == WEOF)
-		  {
-		    err = -1;
-		    goto out;
-		  }
-	      }
-	  }
-	}
-      else
-#endif
-	err = __sfvwrite_r(ptr, fp, uio);
+  err = __sfvwrite_r(ptr, fp, uio);
 out:
   uio->uio_resid = 0;
   uio->uio_iovcnt = 0;
@@ -407,27 +382,11 @@ __sfputs_r (struct _reent *ptr,
 {
   register int i;
 
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-  if (fp->_flags2 & __SWID)
-    {
-      wchar_t *p;
-
-      p = (wchar_t *) buf;
-      for (i = 0; i < (len / sizeof (wchar_t)); i++)
-	{
-	  if (_fputwc_r (ptr, p[i], fp) == WEOF)
-	    return -1;
-	}
-    }
-  else
-#endif
+  for (i = 0; i < len; i++)
     {
-      for (i = 0; i < len; i++)
-	{
-	  /* Call __sfputc_r to skip _fputc_r.  */
-	  if (__sfputc_r (ptr, (int)buf[i], fp) == EOF)
-	    return -1;
-	}
+      /* Call __sfputc_r to skip _fputc_r.  */
+      if (__sfputc_r (ptr, (int)buf[i], fp) == EOF)
+	return -1;
     }
   return (0);
 }
diff --git a/newlib/libc/stdio/vfprintf.c b/newlib/libc/stdio/vfprintf.c
index 6a198e2c6..95412ced9 100644
--- a/newlib/libc/stdio/vfprintf.c
+++ b/newlib/libc/stdio/vfprintf.c
@@ -187,7 +187,7 @@ static char *rcsid = "$Id$";
 # endif
 #endif
 
-/* The __sprint_r/__ssprint_r functions are shared between all versions of
+/* The __ssputs_r/__ssprint_r functions are shared between all versions of
    vfprintf and vfwprintf.  They must only be defined once, which we do in
    the INTEGER_ONLY versions here. */
 #ifdef STRING_ONLY
@@ -370,23 +370,9 @@ __sfputs_r (struct _reent *ptr,
 {
 	register int i;
 
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-	if (fp->_flags2 & __SWID) {
-		wchar_t *p;
-
-		p = (wchar_t *) buf;
-		for (i = 0; i < (len / sizeof (wchar_t)); i++) {
-			if (_fputwc_r (ptr, p[i], fp) == WEOF)
-				return -1;
-		}
-	} else {
-#else
-	{
-#endif
-		for (i = 0; i < len; i++) {
-			if (_fputc_r (ptr, buf[i], fp) == EOF)
-				return -1;
-		}
+	for (i = 0; i < len; i++) {
+		if (_fputc_r (ptr, buf[i], fp) == EOF)
+			return -1;
 	}
 	return (0);
 }
@@ -406,27 +392,7 @@ __sprint_r (struct _reent *ptr,
 		uio->uio_iovcnt = 0;
 		return (0);
 	}
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-	if (fp->_flags2 & __SWID) {
-		struct __siov *iov;
-		wchar_t *p;
-		int i, len;
-
-		iov = uio->uio_iov;
-		for (; uio->uio_resid != 0;
-		     uio->uio_resid -= len * sizeof (wchar_t), iov++) {
-			p = (wchar_t *) iov->iov_base;
-			len = iov->iov_len / sizeof (wchar_t);
-			for (i = 0; i < len; i++) {
-				if (_fputwc_r (ptr, p[i], fp) == WEOF) {
-					err = -1;
-					goto out;
-				}
-			}
-		}
-	} else
-#endif
-		err = __sfvwrite_r(ptr, fp, uio);
+	err = __sfvwrite_r(ptr, fp, uio);
 out:
 	uio->uio_resid = 0;
 	uio->uio_iovcnt = 0;
diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c
index 7807a1229..de494f44a 100644
--- a/newlib/libc/stdio/vfwprintf.c
+++ b/newlib/libc/stdio/vfwprintf.c
@@ -155,18 +155,95 @@ int _VFWPRINTF_R (struct _reent *, FILE *, const wchar_t *, va_list);
 # ifdef STRING_ONLY
 #  define __SPRINT __ssprint_r
 # else
-#  define __SPRINT __sprint_r
+#  define __SPRINT __swprint_r
 # endif
 int __SPRINT (struct _reent *, FILE *, register struct __suio *);
 #else
 # ifdef STRING_ONLY
 #  define __SPRINT __ssputs_r
 # else
-#  define __SPRINT __sfputs_r
+#  define __SPRINT __sfputws_r
 # endif
 int __SPRINT (struct _reent *, FILE *, const char *, size_t);
 #endif
+
 #ifndef STRING_ONLY
+#ifdef INTEGER_ONLY
+#ifndef _FVWRITE_IN_STREAMIO
+int
+__sfputws_r (struct _reent *ptr,
+       FILE *fp,
+       const char *buf,
+       size_t len)
+{
+	register int i;
+
+#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
+	wchar_t *p;
+
+	p = (wchar_t *) buf;
+	for (i = 0; i < (len / sizeof (wchar_t)); i++) {
+		if (_fputwc_r (ptr, p[i], fp) == WEOF)
+			return -1;
+	}
+#else
+	for (i = 0; i < len; i++) {
+		if (_fputc_r (ptr, buf[i], fp) == EOF)
+			return -1;
+	}
+#endif
+	return (0);
+}
+#endif
+/*
+ * Flush out all the vectors defined by the given uio,
+ * then reset it so that it can be reused.
+ */
+int
+__swprint_r (struct _reent *ptr,
+       FILE *fp,
+       register struct __suio *uio)
+{
+	register int err = 0;
+
+	if (uio->uio_resid == 0) {
+		uio->uio_iovcnt = 0;
+		return (0);
+	}
+#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
+	do {
+		struct __siov *iov;
+		wchar_t *p;
+		int i, len;
+
+		iov = uio->uio_iov;
+		for (; uio->uio_resid != 0;
+			 uio->uio_resid -= len * sizeof (wchar_t), iov++) {
+			p = (wchar_t *) iov->iov_base;
+			len = iov->iov_len / sizeof (wchar_t);
+			for (i = 0; i < len; i++) {
+				if (_fputwc_r (ptr, p[i], fp) == WEOF) {
+					err = -1;
+					goto out;
+				}
+			}
+		}
+	} while (0);
+#else
+	err = __sfvwrite_r(ptr, fp, uio);
+#endif
+out:
+	uio->uio_resid = 0;
+	uio->uio_iovcnt = 0;
+	return (err);
+}
+#else /* !INTEGER_ONLY */
+#ifndef _FVWRITE_IN_STREAMIO
+int __sfputws_r (struct _reent *, FILE *, const char *buf, size_t);
+#endif
+int __swprint_r (struct _reent *, FILE *, register struct __suio *);
+#endif /* !INTEGER_ONLY */
+
 #ifdef _UNBUF_STREAM_OPT
 /*
  * Helper function for `fprintf to unbuffered unix file': creates a
-- 
2.39.0