This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix crashes on invalid input in IBM gconv modules [BZ #17325]
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: Florian Weimer <fweimer at redhat dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 29 Aug 2014 09:43:23 -0300
- Subject: Re: [PATCH] Fix crashes on invalid input in IBM gconv modules [BZ #17325]
- Authentication-results: sourceware.org; auth=none
- References: <54006E57 dot 8030800 at redhat dot com>
It is ok, thanks for fixing it.
On 29-08-2014 09:13, Florian Weimer wrote:
> These changes are based on the fix for BZ #14134 in commit 6e230d11837f3ae7b375ea69d7905f0d18eb79e5. My analysis showed that the 0xffff case was impossible with certain character sets, so I added asserts there.
>
> Tested on master with no failures.
>
> I will request CVE IDs (for this fix and the 2012 commit) on oss-security.
>
> --
> Florian Weimer / Red Hat Product Security
>
> From f787724b8a7036804a789737cc0fea040f7b999c Mon Sep 17 00:00:00 2001
> From: Florian Weimer <fweimer@redhat.com>
> Date: Fri, 29 Aug 2014 13:16:49 +0200
> Subject: [PATCH] Fix crashes on invalid input in IBM gconv modules [BZ #17325]
>
> These changes are based on the fix for BZ #14134 in commit
> 6e230d11837f3ae7b375ea69d7905f0d18eb79e5.
>
> 2014-08-29 Florian Weimer <fweimer@redhat.com>
>
> [BZ #17325]
> * iconvdata/ibm1364.c (BODY): Fix check for sentinel.
> * iconvdata/ibm932.c (BODY): Replace invalid sentinel check with
> assert.
> * iconvdata/ibm933.c (BODY): Fix check for sentinel.
> * iconvdata/ibm935.c (BODY): Likewise.
> * iconvdata/ibm937.c (BODY): Likewise.
> * iconvdata/ibm939.c (BODY): Likewise.
> * iconvdata/ibm943.c (BODY): Replace invalid sentinel check with
> assert.
> * iconvdata/Makefile (iconv-test.out): Pass module list to test
> script.
> * iconvdata/run-iconv-test.sh: New test loop for checking for
> decoder crashers.
>
> diff --git a/NEWS b/NEWS
> index 1af9e70..dd93f2f 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -23,7 +23,7 @@ Version 2.20
> 16966, 16967, 16977, 16978, 16984, 16990, 16996, 17009, 17022, 17031,
> 17042, 17048, 17050, 17058, 17061, 17062, 17069, 17075, 17078, 17079,
> 17084, 17086, 17088, 17092, 17097, 17125, 17135, 17137, 17150, 17153,
> - 17187, 17213, 17259, 17261, 17262, 17263, 17319.
> + 17187, 17213, 17259, 17261, 17262, 17263, 17319, 17325.
>
> * Reverted change of ABI data structures for s390 and s390x:
> On s390 and s390x the size of struct ucontext and jmp_buf was increased in
> @@ -115,6 +115,11 @@ Version 2.20
> normal gconv conversion modules are still supported. Transliteration
> with //TRANSLIT is still possible, and the //IGNORE specifier
> continues to be supported. (CVE-2014-5119)
> +
> +* Decoding a crafted input sequence in the character sets IBM933, IBM935,
> + IBM937, IBM939, IBM1364 could result in an out-of-bounds array read,
> + resulting a denial-of-service security vulnerability in applications which
> + use functions related to iconv.
>
> Version 2.19
>
> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
> index 0a410a1..b6327d6 100644
> --- a/iconvdata/Makefile
> +++ b/iconvdata/Makefile
> @@ -297,6 +297,7 @@ $(objpfx)tst-iconv7.out: $(objpfx)gconv-modules \
> $(objpfx)iconv-test.out: run-iconv-test.sh $(objpfx)gconv-modules \
> $(addprefix $(objpfx),$(modules.so)) \
> $(common-objdir)/iconv/iconv_prog TESTS
> + iconv_modules="$(modules)" \
> $(SHELL) $< $(common-objdir) '$(test-wrapper-env)' \
> '$(run-program-env)' > $@; \
> $(evaluate-test)
> diff --git a/iconvdata/ibm1364.c b/iconvdata/ibm1364.c
> index 0b5484f..cf80993 100644
> --- a/iconvdata/ibm1364.c
> +++ b/iconvdata/ibm1364.c
> @@ -221,7 +221,8 @@ enum
> ++rp2; \
> \
> uint32_t res; \
> - if (__builtin_expect (ch < rp2->start, 0) \
> + if (__builtin_expect (rp2->start == 0xffff, 0) \
> + || __builtin_expect (ch < rp2->start, 0) \
> || (res = DB_TO_UCS4[ch + rp2->idx], \
> __builtin_expect (res, L'\1') == L'\0' && ch != '\0')) \
> { \
> diff --git a/iconvdata/ibm932.c b/iconvdata/ibm932.c
> index f5dca59..aa69d65 100644
> --- a/iconvdata/ibm932.c
> +++ b/iconvdata/ibm932.c
> @@ -74,11 +74,12 @@
> } \
> \
> ch = (ch * 0x100) + inptr[1]; \
> + /* ch was less than 0xfd. */ \
> + assert (ch < 0xfd00); \
> while (ch > rp2->end) \
> ++rp2; \
> \
> - if (__builtin_expect (rp2 == NULL, 0) \
> - || __builtin_expect (ch < rp2->start, 0) \
> + if (__builtin_expect (ch < rp2->start, 0) \
> || (res = __ibm932db_to_ucs4[ch + rp2->idx], \
> __builtin_expect (res, '\1') == 0 && ch !=0)) \
> { \
> diff --git a/iconvdata/ibm933.c b/iconvdata/ibm933.c
> index f46dfb5..461fb5e 100644
> --- a/iconvdata/ibm933.c
> +++ b/iconvdata/ibm933.c
> @@ -162,7 +162,7 @@ enum
> while (ch > rp2->end) \
> ++rp2; \
> \
> - if (__builtin_expect (rp2 == NULL, 0) \
> + if (__builtin_expect (rp2->start == 0xffff, 0) \
> || __builtin_expect (ch < rp2->start, 0) \
> || (res = __ibm933db_to_ucs4[ch + rp2->idx], \
> __builtin_expect (res, L'\1') == L'\0' && ch != '\0')) \
> diff --git a/iconvdata/ibm935.c b/iconvdata/ibm935.c
> index a8e4e6c..56babb8 100644
> --- a/iconvdata/ibm935.c
> +++ b/iconvdata/ibm935.c
> @@ -162,7 +162,7 @@ enum
> while (ch > rp2->end) \
> ++rp2; \
> \
> - if (__builtin_expect (rp2 == NULL, 0) \
> + if (__builtin_expect (ch == 0xffff, 0) \
> || __builtin_expect (ch < rp2->start, 0) \
> || (res = __ibm935db_to_ucs4[ch + rp2->idx], \
> __builtin_expect (res, L'\1') == L'\0' && ch != '\0')) \
> diff --git a/iconvdata/ibm937.c b/iconvdata/ibm937.c
> index 239be61..1ba340f 100644
> --- a/iconvdata/ibm937.c
> +++ b/iconvdata/ibm937.c
> @@ -162,7 +162,7 @@ enum
> while (ch > rp2->end) \
> ++rp2; \
> \
> - if (__builtin_expect (rp2 == NULL, 0) \
> + if (__builtin_expect (ch == 0xffff, 0) \
> || __builtin_expect (ch < rp2->start, 0) \
> || (res = __ibm937db_to_ucs4[ch + rp2->idx], \
> __builtin_expect (res, L'\1') == L'\0' && ch != '\0')) \
> diff --git a/iconvdata/ibm939.c b/iconvdata/ibm939.c
> index 5d0db36..16caa62 100644
> --- a/iconvdata/ibm939.c
> +++ b/iconvdata/ibm939.c
> @@ -162,7 +162,7 @@ enum
> while (ch > rp2->end) \
> ++rp2; \
> \
> - if (__builtin_expect (rp2 == NULL, 0) \
> + if (__builtin_expect (ch == 0xffff, 0) \
> || __builtin_expect (ch < rp2->start, 0) \
> || (res = __ibm939db_to_ucs4[ch + rp2->idx], \
> __builtin_expect (res, L'\1') == L'\0' && ch != '\0')) \
> diff --git a/iconvdata/ibm943.c b/iconvdata/ibm943.c
> index be0c14f..c5d5742 100644
> --- a/iconvdata/ibm943.c
> +++ b/iconvdata/ibm943.c
> @@ -75,11 +75,12 @@
> } \
> \
> ch = (ch * 0x100) + inptr[1]; \
> + /* ch was less than 0xfd. */ \
> + assert (ch < 0xfd00); \
> while (ch > rp2->end) \
> ++rp2; \
> \
> - if (__builtin_expect (rp2 == NULL, 0) \
> - || __builtin_expect (ch < rp2->start, 0) \
> + if (__builtin_expect (ch < rp2->start, 0) \
> || (res = __ibm943db_to_ucs4[ch + rp2->idx], \
> __builtin_expect (res, '\1') == 0 && ch !=0)) \
> { \
> diff --git a/iconvdata/run-iconv-test.sh b/iconvdata/run-iconv-test.sh
> index c98c929..5dfb69f 100755
> --- a/iconvdata/run-iconv-test.sh
> +++ b/iconvdata/run-iconv-test.sh
> @@ -184,6 +184,24 @@ while read utf8 from filename; do
>
> done < TESTS2
>
> +# Check for crashes in decoders.
> +printf '\016\377\377\377\377\377\377\377' > $temp1
> +for from in $iconv_modules ; do
> + echo $ac_n "test decoder $from $ac_c"
> + PROG=`eval echo $ICONV`
> + if $PROG < $temp1 >/dev/null 2>&1 ; then
> + : # fall through
> + else
> + status=$?
> + if test $status -gt 1 ; then
> + echo "/FAILED"
> + failed=1
> + continue
> + fi
> + fi
> + echo "OK"
> +done
> +
> exit $failed
> # Local Variables:
> # mode:shell-script
> -- 1.9.3