This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/2 v1.1][BZ #14547] Fix CVE-2012-4412
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Siddhesh Poyarekar <siddhesh dot poyarekar at gmail dot com>
- Cc: Siddhesh Poyarekar <siddhesh at redhat dot com>, Andreas Schwab <schwab at suse dot de>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Sat, 24 Aug 2013 08:22:48 +0200
- Subject: Re: [PATCH 2/2 v1.1][BZ #14547] Fix CVE-2012-4412
- 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>
On Wed, Aug 21, 2013 at 11:41:34PM +0530, Siddhesh Poyarekar wrote:
> On 21 August 2013 21:38, OndÅej BÃlka <neleai@seznam.cz> wrote:
> > Is that goto needed? I would add conditions to if like following:
> >
> > if (!MIN (s1len, s2len) > size_max
> > && !MAX (s1len, s2len) > size_max - MIN (s1len, s2len)
> > && !__libc_use_alloca ((s1len + s2len) * (sizeof (int32_t) + 1)))
> >
> > There I assume that if below is single block, otherwise I would add new block.
>
> It eliminates nesting and makes code easier to read. Not all goto is evil.
>
Not all but this is. If you do not want nesting use following:
diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index eb042ff..f76c30a 100644
--- a/string/strcoll_l.c
+++ b/string/strcoll_l.c
@@ -523,8 +523,13 @@ 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)))
+ 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));