Fix CVE-2020-29562, Fix incorrect UCS4 inner loop bounds (BZ#26923)

This commit is contained in:
wangshuo 2020-12-21 14:55:32 +08:00 committed by liqingqing_1229
parent deff0486a7
commit 2793651bb6
3 changed files with 502 additions and 1 deletions

View File

@ -0,0 +1,155 @@
From 228edd356f03bf62dcf2b1335f25d43c602ee68d Mon Sep 17 00:00:00 2001
From: Michael Colavita <mcolavita@fb.com>
Date: Thu, 19 Nov 2020 11:44:40 -0500
Subject: [PATCH] iconv: Fix incorrect UCS4 inner loop bounds (BZ#26923)
reason:Fix incorrect UCS4 inner loop bounds
Conflict:NA
Reference:https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-29562
Previously, in UCS4 conversion routines we limit the number of
characters we examine to the minimum of the number of characters in the
input and the number of characters in the output. This is not the
correct behavior when __GCONV_IGNORE_ERRORS is set, as we do not consume
an output character when we skip a code unit. Instead, track the input
and output pointers and terminate the loop when either reaches its
limit.
This resolves assertion failures when resetting the input buffer in a step of
iconv, which assumes that the input will be fully consumed given sufficient
output space.
---
iconv/Makefile | 2 +-
iconv/gconv_simple.c | 16 ++++----------
iconv/tst-iconv8.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 55 insertions(+), 13 deletions(-)
create mode 100644 iconv/tst-iconv8.c
diff --git a/iconv/Makefile b/iconv/Makefile
index 73d983af..54fed214 100644
--- a/iconv/Makefile
+++ b/iconv/Makefile
@@ -44,7 +44,7 @@ CFLAGS-linereader.c += -DNO_TRANSLITERATION
CFLAGS-simple-hash.c += -I../locale
tests = tst-iconv1 tst-iconv2 tst-iconv3 tst-iconv4 tst-iconv5 tst-iconv6 \
- tst-iconv7
+ tst-iconv7 tst-iconv8
others = iconv_prog iconvconfig
others-pie += iconv_prog iconvconfig
diff --git a/iconv/gconv_simple.c b/iconv/gconv_simple.c
index c487b892..f47d56f4 100644
--- a/iconv/gconv_simple.c
+++ b/iconv/gconv_simple.c
@@ -239,11 +239,9 @@ ucs4_internal_loop (struct __gconv_step *step,
int flags = step_data->__flags;
const unsigned char *inptr = *inptrp;
unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
int result;
- size_t cnt;
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
+ for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
uint32_t inval;
@@ -307,11 +305,9 @@ ucs4_internal_loop_unaligned (struct __gconv_step *step,
int flags = step_data->__flags;
const unsigned char *inptr = *inptrp;
unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
int result;
- size_t cnt;
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
+ for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
if (__glibc_unlikely (inptr[0] > 0x80))
{
@@ -613,11 +609,9 @@ ucs4le_internal_loop (struct __gconv_step *step,
int flags = step_data->__flags;
const unsigned char *inptr = *inptrp;
unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
int result;
- size_t cnt;
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
+ for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
uint32_t inval;
@@ -684,11 +678,9 @@ ucs4le_internal_loop_unaligned (struct __gconv_step *step,
int flags = step_data->__flags;
const unsigned char *inptr = *inptrp;
unsigned char *outptr = *outptrp;
- size_t n_convert = MIN (inend - inptr, outend - outptr) / 4;
int result;
- size_t cnt;
- for (cnt = 0; cnt < n_convert; ++cnt, inptr += 4)
+ for (; inptr + 4 <= inend && outptr + 4 <= outend; inptr += 4)
{
if (__glibc_unlikely (inptr[3] > 0x80))
{
diff --git a/iconv/tst-iconv8.c b/iconv/tst-iconv8.c
new file mode 100644
index 00000000..0b92b19f
--- /dev/null
+++ b/iconv/tst-iconv8.c
@@ -0,0 +1,50 @@
+/* Test iconv behavior on UCS4 conversions with //IGNORE.
+ Copyright (C) 2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* Derived from BZ #26923 */
+#include <errno.h>
+#include <iconv.h>
+#include <stdio.h>
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+ iconv_t cd = iconv_open ("UTF-8//IGNORE", "ISO-10646/UCS4/");
+ TEST_VERIFY_EXIT (cd != (iconv_t) -1);
+
+ /*
+ * Convert sequence beginning with an irreversible character into buffer that
+ * is too small.
+ */
+ char input[12] = "\xe1\x80\xa1" "AAAAAAAAA";
+ char *inptr = input;
+ size_t insize = sizeof (input);
+ char output[6];
+ char *outptr = output;
+ size_t outsize = sizeof (output);
+
+ TEST_VERIFY (iconv (cd, &inptr, &insize, &outptr, &outsize) == -1);
+ TEST_VERIFY (errno == E2BIG);
+
+ TEST_VERIFY_EXIT (iconv_close (cd) != -1);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.23.0

View File

@ -0,0 +1,339 @@
From 4802be92c891903caaf8cae47f685da6f26d4b9a Mon Sep 17 00:00:00 2001
From: Andreas Schwab <schwab@suse.de>
Date: Mon, 17 Aug 2015 14:05:01 +0200
Subject: [PATCH] Fix iconv buffer handling with IGNORE error handler (bug
#18830)
reason:Fix iconv buffer handling with IGNORE error handler
Conflict:NA
Reference:https://sourceware.org/bugzilla/show_bug.cgi?id=18830
---
iconv/Makefile | 3 +-
iconv/gconv_simple.c | 32 ++++++++++------
iconv/skeleton.c | 35 +++++++++++++----
iconv/tst-iconv7.c | 55 +++++++++++++++++++++++++++
sysdeps/s390/multiarch/gconv_simple.c | 6 +--
5 files changed, 108 insertions(+), 23 deletions(-)
create mode 100644 iconv/tst-iconv7.c
diff --git a/iconv/Makefile b/iconv/Makefile
index e4d73376..73d983af 100644
--- a/iconv/Makefile
+++ b/iconv/Makefile
@@ -43,7 +43,8 @@ CFLAGS-charmap.c += -DCHARMAP_PATH='"$(i18ndir)/charmaps"' \
CFLAGS-linereader.c += -DNO_TRANSLITERATION
CFLAGS-simple-hash.c += -I../locale
-tests = tst-iconv1 tst-iconv2 tst-iconv3 tst-iconv4 tst-iconv5 tst-iconv6
+tests = tst-iconv1 tst-iconv2 tst-iconv3 tst-iconv4 tst-iconv5 tst-iconv6 \
+ tst-iconv7
others = iconv_prog iconvconfig
others-pie += iconv_prog iconvconfig
diff --git a/iconv/gconv_simple.c b/iconv/gconv_simple.c
index 506c92ca..c487b892 100644
--- a/iconv/gconv_simple.c
+++ b/iconv/gconv_simple.c
@@ -76,7 +76,7 @@ __attribute ((always_inline))
internal_ucs4_loop (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp, const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp, const unsigned char *outend,
size_t *irreversible)
{
const unsigned char *inptr = *inptrp;
@@ -120,7 +120,8 @@ internal_ucs4_loop_unaligned (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
const unsigned char *inptr = *inptrp;
@@ -169,7 +170,8 @@ internal_ucs4_loop_single (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
mbstate_t *state = step_data->__statep;
@@ -231,7 +233,7 @@ __attribute ((always_inline))
ucs4_internal_loop (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp, const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp, const unsigned char *outend,
size_t *irreversible)
{
int flags = step_data->__flags;
@@ -298,7 +300,8 @@ ucs4_internal_loop_unaligned (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
int flags = step_data->__flags;
@@ -368,7 +371,8 @@ ucs4_internal_loop_single (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
mbstate_t *state = step_data->__statep;
@@ -443,7 +447,7 @@ __attribute ((always_inline))
internal_ucs4le_loop (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp, const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp, const unsigned char *outend,
size_t *irreversible)
{
const unsigned char *inptr = *inptrp;
@@ -488,7 +492,8 @@ internal_ucs4le_loop_unaligned (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
const unsigned char *inptr = *inptrp;
@@ -540,7 +545,8 @@ internal_ucs4le_loop_single (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
mbstate_t *state = step_data->__statep;
@@ -601,7 +607,7 @@ __attribute ((always_inline))
ucs4le_internal_loop (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp, const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp, const unsigned char *outend,
size_t *irreversible)
{
int flags = step_data->__flags;
@@ -671,7 +677,8 @@ ucs4le_internal_loop_unaligned (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
int flags = step_data->__flags;
@@ -745,7 +752,8 @@ ucs4le_internal_loop_single (struct __gconv_step *step,
struct __gconv_step_data *step_data,
const unsigned char **inptrp,
const unsigned char *inend,
- unsigned char **outptrp, unsigned char *outend,
+ unsigned char **outptrp,
+ const unsigned char *outend,
size_t *irreversible)
{
mbstate_t *state = step_data->__statep;
diff --git a/iconv/skeleton.c b/iconv/skeleton.c
index fa799305..f4e499e2 100644
--- a/iconv/skeleton.c
+++ b/iconv/skeleton.c
@@ -83,6 +83,11 @@
RESET_INPUT_BUFFER If the input character sets allow this the macro
can be defined to reset the input buffer pointers
to cover only those characters up to the error.
+ Note that if the conversion has skipped over
+ irreversible characters (due to
+ __GCONV_IGNORE_ERRORS) there is no longer a direct
+ correspondence between input and output pointers,
+ and this macro is not called.
FUNCTION_NAME if not set the conversion function is named `gconv'.
@@ -597,6 +602,12 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
inptr = *inptrp;
/* The outbuf buffer is empty. */
outstart = outbuf;
+#ifdef RESET_INPUT_BUFFER
+ /* Remember how many irreversible characters were skipped before
+ this round. */
+ size_t loop_irreversible
+ = lirreversible + (irreversible ? *irreversible : 0);
+#endif
#ifdef SAVE_RESET_STATE
SAVE_RESET_STATE (1);
@@ -671,8 +682,16 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
if (__glibc_unlikely (outerr != outbuf))
{
#ifdef RESET_INPUT_BUFFER
- RESET_INPUT_BUFFER;
-#else
+ /* RESET_INPUT_BUFFER can only work when there were
+ no new irreversible characters skipped during
+ this round. */
+ if (loop_irreversible
+ == lirreversible + (irreversible ? *irreversible : 0))
+ {
+ RESET_INPUT_BUFFER;
+ goto done_reset;
+ }
+#endif
/* We have a problem in one of the functions below.
Undo the conversion upto the error point. */
size_t nstatus __attribute__ ((unused));
@@ -682,9 +701,9 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
outbuf = outstart;
/* Restore the state. */
-# ifdef SAVE_RESET_STATE
+#ifdef SAVE_RESET_STATE
SAVE_RESET_STATE (0);
-# endif
+#endif
if (__glibc_likely (!unaligned))
{
@@ -701,7 +720,7 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
lirreversiblep
EXTRA_LOOP_ARGS);
}
-# if POSSIBLY_UNALIGNED
+#if POSSIBLY_UNALIGNED
else
{
if (FROM_DIRECTION)
@@ -720,7 +739,7 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
lirreversiblep
EXTRA_LOOP_ARGS);
}
-# endif
+#endif
/* We must run out of output buffer space in this
rerun. */
@@ -731,9 +750,11 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data,
the invocation counter. */
if (__glibc_unlikely (outbuf == outstart))
--data->__invocation_counter;
-#endif /* reset input buffer */
}
+#ifdef RESET_INPUT_BUFFER
+ done_reset:
+#endif
/* Change the status. */
status = result;
}
diff --git a/iconv/tst-iconv7.c b/iconv/tst-iconv7.c
new file mode 100644
index 00000000..10372bf7
--- /dev/null
+++ b/iconv/tst-iconv7.c
@@ -0,0 +1,55 @@
+/* Test iconv buffer handling with the IGNORE error handler.
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* Derived from BZ #18830 */
+#include <errno.h>
+#include <iconv.h>
+#include <stdio.h>
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+ /* This conversion needs two steps, from ASCII to INTERNAL to ASCII. */
+ iconv_t cd = iconv_open ("ASCII//IGNORE", "ASCII");
+ TEST_VERIFY_EXIT (cd != (iconv_t) -1);
+
+ /* Convert some irreversible sequence, enough to trigger an overflow of
+ the output buffer before the irreversible character in the second
+ step, but after going past the irreversible character in the first
+ step. */
+ char input[4 + 4] = { '0', '1', '2', '3', '4', '5', '\266', '7' };
+ char *inptr = input;
+ size_t insize = sizeof (input);
+ char output[4];
+ char *outptr = output;
+ size_t outsize = sizeof (output);
+
+ /* The conversion should fail. */
+ TEST_VERIFY (iconv (cd, &inptr, &insize, &outptr, &outsize) == (size_t) -1);
+ TEST_VERIFY (errno == E2BIG);
+ /* The conversion should not consume more than it was able to store in
+ the output buffer. */
+ TEST_COMPARE (inptr - input, sizeof (output) - outsize);
+
+ TEST_VERIFY_EXIT (iconv_close (cd) != -1);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/s390/multiarch/gconv_simple.c b/sysdeps/s390/multiarch/gconv_simple.c
index aaa1ebf7..88772fab 100644
--- a/sysdeps/s390/multiarch/gconv_simple.c
+++ b/sysdeps/s390/multiarch/gconv_simple.c
@@ -403,7 +403,7 @@ ICONV_VX_NAME (internal_ucs4le_loop) (struct __gconv_step *step,
const unsigned char **inptrp,
const unsigned char *inend,
unsigned char **outptrp,
- unsigned char *outend,
+ const unsigned char *outend,
size_t *irreversible)
{
const unsigned char *inptr = *inptrp;
@@ -503,7 +503,7 @@ ICONV_VX_NAME (ucs4_internal_loop) (struct __gconv_step *step,
const unsigned char **inptrp,
const unsigned char *inend,
unsigned char **outptrp,
- unsigned char *outend,
+ const unsigned char *outend,
size_t *irreversible)
{
int flags = step_data->__flags;
@@ -630,7 +630,7 @@ ICONV_VX_NAME (ucs4le_internal_loop) (struct __gconv_step *step,
const unsigned char **inptrp,
const unsigned char *inend,
unsigned char **outptrp,
- unsigned char *outend,
+ const unsigned char *outend,
size_t *irreversible)
{
int flags = step_data->__flags;
--
2.23.0

View File

@ -59,7 +59,7 @@
##############################################################################
Name: glibc
Version: 2.28
Release: 48
Release: 49
Summary: The GNU libc libraries
License: %{all_license}
URL: http://www.gnu.org/software/glibc/
@ -99,6 +99,8 @@ Patch22: backport-i686-tst-strftime3-fix-printf-warning.patch
Patch23: Fix-CVE-2020-27618-iconv-Accept-redundant-shift-sequences.patch
Patch24: backport-x86-Use-one-ldbl2mpn.c-file-for-both-i386-and-x86_64.patch
Patch25: backport-Fix-CVE-2020-29573-x86-Harden-printf-against-non-normal-long-double-val.patch
Patch26: backport-Fix-iconv-buffer-handling-with-IGNORE-error-handler-.patch
Patch27: backport-CVE-2020-29562-iconv-Fix-incorrect-UCS4-inner-loop-bounds-BZ-26923.patch
Provides: ldconfig rtld(GNU_HASH) bundled(gnulib)
@ -1094,6 +1096,11 @@ fi
%doc hesiod/README.hesiod
%changelog
* Mon Dec 21 2020 Wang Shuo<wangshuo_1994@foxmail.com> - 2.28-49
- Fix CVE-2020-29562, Fix incorrect UCS4 inner loop bounds (BZ#26923)
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-29562
https://sourceware.org/bugzilla/show_bug.cgi?id=26923
* Thu Dec 17 2020 Wang Shuo <wangshuo_1994@foxmail.com> - 2.28-48
- Fix CVE-2020-29573, Harden printf against non-normal long double values
https://cve.mitre.org/cgi-bin/cvename.cgi?name=2020-29573