115 lines
3.6 KiB
Diff
115 lines
3.6 KiB
Diff
|
From 3ab2a4e02682df1382955071919d8aa3c3ec40d4 Mon Sep 17 00:00:00 2001
|
||
|
From: Rich Felker <dalias@aerifal.cx>
|
||
|
Date: Thu, 19 Nov 2020 17:12:43 -0500
|
||
|
Subject: [PATCH] rewrite wcsnrtombs to fix buffer overflow and other bugs
|
||
|
|
||
|
the original wcsnrtombs implementation, which has been largely
|
||
|
untouched since 0.5.0, attempted to build input-length-limiting
|
||
|
conversion on top of wcsrtombs, which only limits output length. as
|
||
|
best I recall, this choice was made out of a mix of disdain over
|
||
|
having yet another variant function to implement (added in POSIX 2008;
|
||
|
not standard C) and preference not to switch things around and
|
||
|
implement the wcsrtombs in terms of the more general new function,
|
||
|
probably over namespace issues. the strategy employed was to impose
|
||
|
output limits that would ensure the input limit wasn't exceeded, then
|
||
|
finish up the tail character-at-a-time. unfortunately, none of that
|
||
|
worked correctly.
|
||
|
|
||
|
first, the logic in the wcsrtombs loop was wrong in that it could
|
||
|
easily get stuck making no forward progress, by imposing an output
|
||
|
limit too small to convert even one character.
|
||
|
|
||
|
the character-at-a-time loop that followed was even worse. it made no
|
||
|
effort to ensure that the converted multibyte character would fit in
|
||
|
the remaining output space, only that there was a nonzero amount of
|
||
|
output space remaining. it also employed an incorrect interpretation
|
||
|
of wcrtomb's interface contract for converting the null character,
|
||
|
thereby failing to act on end of input, and remaining space accounting
|
||
|
was subject to unsigned wrap-around. together these errors allow
|
||
|
unbounded overflow of the destination buffer, controlled by input
|
||
|
length limit and input wchar_t string contents.
|
||
|
|
||
|
given the extent to which this function was broken, it's plausible
|
||
|
that most applications that would have been rendered exploitable were
|
||
|
sufficiently broken not to be usable in the first place. however, it's
|
||
|
also plausible that common (especially ASCII-only) inputs succeeded in
|
||
|
the wcsrtombs loop, which mostly worked, while leaving the wildly
|
||
|
erroneous code in the second loop exposed to particular non-ASCII
|
||
|
inputs.
|
||
|
|
||
|
CVE-2020-28928 has been assigned for this issue.
|
||
|
|
||
|
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
|
||
|
---
|
||
|
src/multibyte/wcsnrtombs.c | 46 ++++++++++++++++----------------------
|
||
|
1 file changed, 19 insertions(+), 27 deletions(-)
|
||
|
|
||
|
diff --git a/src/multibyte/wcsnrtombs.c b/src/multibyte/wcsnrtombs.c
|
||
|
index 676932b5..95e25e70 100644
|
||
|
--- a/src/multibyte/wcsnrtombs.c
|
||
|
+++ b/src/multibyte/wcsnrtombs.c
|
||
|
@@ -1,41 +1,33 @@
|
||
|
#include <wchar.h>
|
||
|
+#include <limits.h>
|
||
|
+#include <string.h>
|
||
|
|
||
|
size_t wcsnrtombs(char *restrict dst, const wchar_t **restrict wcs, size_t wn, size_t n, mbstate_t *restrict st)
|
||
|
{
|
||
|
- size_t l, cnt=0, n2;
|
||
|
- char *s, buf[256];
|
||
|
const wchar_t *ws = *wcs;
|
||
|
- const wchar_t *tmp_ws;
|
||
|
-
|
||
|
- if (!dst) s = buf, n = sizeof buf;
|
||
|
- else s = dst;
|
||
|
-
|
||
|
- while ( ws && n && ( (n2=wn)>=n || n2>32 ) ) {
|
||
|
- if (n2>=n) n2=n;
|
||
|
- tmp_ws = ws;
|
||
|
- l = wcsrtombs(s, &ws, n2, 0);
|
||
|
- if (!(l+1)) {
|
||
|
- cnt = l;
|
||
|
- n = 0;
|
||
|
+ size_t cnt = 0;
|
||
|
+ if (!dst) n=0;
|
||
|
+ while (ws && wn) {
|
||
|
+ char tmp[MB_LEN_MAX];
|
||
|
+ size_t l = wcrtomb(n<MB_LEN_MAX ? tmp : dst, *ws, 0);
|
||
|
+ if (l==-1) {
|
||
|
+ cnt = -1;
|
||
|
break;
|
||
|
}
|
||
|
- if (s != buf) {
|
||
|
- s += l;
|
||
|
+ if (dst) {
|
||
|
+ if (n<MB_LEN_MAX) {
|
||
|
+ if (l>n) break;
|
||
|
+ memcpy(dst, tmp, l);
|
||
|
+ }
|
||
|
+ dst += l;
|
||
|
n -= l;
|
||
|
}
|
||
|
- wn = ws ? wn - (ws - tmp_ws) : 0;
|
||
|
- cnt += l;
|
||
|
- }
|
||
|
- if (ws) while (n && wn) {
|
||
|
- l = wcrtomb(s, *ws, 0);
|
||
|
- if ((l+1)<=1) {
|
||
|
- if (!l) ws = 0;
|
||
|
- else cnt = l;
|
||
|
+ if (!*ws) {
|
||
|
+ ws = 0;
|
||
|
break;
|
||
|
}
|
||
|
- ws++; wn--;
|
||
|
- /* safe - this loop runs fewer than sizeof(buf) times */
|
||
|
- s+=l; n-=l;
|
||
|
+ ws++;
|
||
|
+ wn--;
|
||
|
cnt += l;
|
||
|
}
|
||
|
if (dst) *wcs = ws;
|
||
|
--
|
||
|
2.20.1
|
||
|
|