This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH 2/2 v1.2][BZ #14547] Fix CVE-2012-4412
- From: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: Paul Eggert <eggert at cs dot ucla dot edu>
- Cc: Siddhesh Poyarekar <siddhesh dot poyarekar at gmail dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 26 Aug 2013 09:14:16 +0530
- Subject: [PATCH 2/2 v1.2][BZ #14547] Fix CVE-2012-4412
- Authentication-results: sourceware.org; auth=none
- References: <20130630164500 dot GF2654 at spoyarek dot pnq dot redhat dot com> <mvmehagsfhm dot fsf at hawking dot suse dot de> <20130821151403 dot GB15273 at spoyarek dot pnq dot redhat dot com> <20130821160808 dot GA4369 at domone dot kolej dot mff dot cuni dot cz> <CAAHN_R2UwiuNAXyqv7QvZzkbfy7vqAMn1EQ-ka82OFUogChVvQ at mail dot gmail dot com> <20130824062248 dot GA16906 at domone dot kolej dot mff dot cuni dot cz> <CAAHN_R2UYzo+N3gcPjUzmkQFOBj6mmKZ2hbHbJW9cfeFthXw7A at mail dot gmail dot com> <5218C3AC dot 1060504 at cs dot ucla dot edu>
On Sat, Aug 24, 2013 at 07:31:08AM -0700, Paul Eggert wrote:
> The empty if-block is a bit easier for me to read than the goto.
> I got the willies when I saw the goto over the declaration;
> this goto happens to be valid, but it's just one more silly
> thing to worry about.
Fair enough - two against one on personal preference (and I don't care
enough to wait longer), so here's an updated version.
Siddhesh
[BZ #14547]
* string/tst-strcoll-overflow.c: New test case.
* string/Makefile (xtests): Add tst-strcoll-overflow.
* string/strcoll_l.c (STRCOLL): Skip allocating memory for
cache if string sizes may cause integer overflow.
diff --git a/string/Makefile b/string/Makefile
index 0237edd..59c658f 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -57,6 +57,8 @@ tests := tester inl-tester noinl-tester testcopy test-ffs \
tests-ifunc := $(strop-tests:%=test-%-ifunc)
tests += $(tests-ifunc)
+xtests = tst-strcoll-overflow
+
include ../Rules
tester-ENV = LANGUAGE=C
diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index eb042ff..4ee101a 100644
--- a/string/strcoll_l.c
+++ b/string/strcoll_l.c
@@ -524,7 +524,15 @@ STRCOLL (const STRING_TYPE *s1, const STRING_TYPE *s2, __locale_t l)
memset (&seq1, 0, sizeof (seq1));
seq2 = seq1;
- if (! __libc_use_alloca ((s1len + s2len) * (sizeof (int32_t) + 1)))
+ size_t size_max = SIZE_MAX / (sizeof (int32_t) + 1);
+
+ if (MIN (s1len, s2len) > size_max
+ || MAX (s1len, s2len) > size_max - MIN (s1len, s2len))
+ {
+ /* If the strings are long enough to cause overflow in the size request,
+ then skip the allocation and proceed with the non-cached routines. */
+ }
+ else if (! __libc_use_alloca ((s1len + s2len) * (sizeof (int32_t) + 1)))
{
seq1.idxarr = (int32_t *) malloc ((s1len + s2len) * (sizeof (int32_t) + 1));
diff --git a/string/tst-strcoll-overflow.c b/string/tst-strcoll-overflow.c
new file mode 100644
index 0000000..bb665ac
--- /dev/null
+++ b/string/tst-strcoll-overflow.c
@@ -0,0 +1,61 @@
+/* Copyright (C) 2013 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/>. */
+
+#include <locale.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+
+/* Verify that strcoll does not crash for large strings for which it cannot
+ cache weight lookup results. The size is large enough to cause integer
+ overflows on 32-bit as well as buffer overflows on 64-bit. The test should
+ work reasonably reliably when overcommit is disabled, but it obviously
+ depends on how much memory the system has. There's a limitation to this
+ test in that it does not run to completion. Actually collating such a
+ large string can take days and we can't have xcheck running that long. For
+ that reason, we run the test for about 5 minutes and then assume that
+ everything is fine if there are no crashes. */
+#define SIZE 0x40000000ul
+
+int
+do_test (void)
+{
+ if (setlocale (LC_COLLATE, "en_GB.UTF-8") == NULL)
+ {
+ puts ("setlocale failed, cannot test for overflow");
+ return 0;
+ }
+
+ char *p = malloc (SIZE);
+
+ if (p == NULL)
+ {
+ puts ("could not allocate memory");
+ return 1;
+ }
+
+ memset (p, 'x', SIZE - 1);
+ p[SIZE - 1] = 0;
+ printf ("%d\n", strcoll (p, p));
+ return 0;
+}
+
+#define TIMEOUT 300
+#define EXPECTED_SIGNAL SIGALRM
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"