Check for integer overflow in cache size computation in strcoll

strcoll is implemented using a cache for indices and weights of
collation sequences in the strings so that subsequent passes do not
have to search through collation data again.  For very large string
inputs, the cache size computation could overflow.  In such a case,
use the fallback function that does not cache indices and weights of
collation sequences.

Fixes CVE-2012-4412.

diff --git a/ChangeLog b/ChangeLog
index 6886e5f2f2e9107abece2c0c885209e8669979e3..2a11ed87995613916110d80e7bbb8cfeb1a68312 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2013-09-23  Siddhesh Poyarekar  <siddhesh@redhat.com>
 
+       [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.
+
        [BZ #14547]
        * string/strcoll_l.c (coll_seq): New members rule, idx,
        save_idx and back_us.

diff --git a/NEWS b/NEWS
index aec8b0048e6b9da1e1b17cd8bc2a2d1c76a07290..e83c78c285dc9076757ca0c516ce812364e8b2da 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -45,6 +45,12 @@ Version 2.18
   15655, 15666, 15667, 15674, 15711, 15755, 15759, 15797, 15892, 15893,
   15895.
 
+* CVE-2012-4412 The strcoll implementation caches indices and rules for
+  large collation sequences to optimize multiple passes.  This cache
+  computation may overflow for large collation sequences and may cause a
+  stack or buffer overflow.  This is now fixed to use a slower algorithm
+  which does not use a cache if there is an integer overflow.
+
 * CVE-2013-2207 Incorrectly granting access to another user's pseudo-terminal
   has been fixed by disabling the use of pt_chown (Bugzilla #15755).
   Distributions can re-enable building and using pt_chown via the new configure

diff --git a/string/Makefile b/string/Makefile
index 72d3e29f57c10ea1591b74f3d2b3d5818b3d304c..17f9d68614ba846abb9ccff1bf9ff2afc6f272d8 100644 (file)
--- 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 eb042ff2ecad9fe03e491d9dc663edcb39209a41..4ee101a118a15194f8ca0e3cbdc53e2d5aa483f8 100644 (file)
--- 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 (file)
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"