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 at redhat dot com>
- Cc: Andreas Schwab <schwab at suse dot de>, libc-alpha at sourceware dot org
- Date: Wed, 21 Aug 2013 18:08:08 +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>
On Wed, Aug 21, 2013 at 08:44:03PM +0530, Siddhesh Poyarekar wrote:
> + size_t size_max = SIZE_MAX / (sizeof (int32_t) + 1);
> +
> + /* If the strings are long enough to cause overflow in the size request, then
> + skip the allocation and proceed with the non-cached routines. */
> + if (MIN (s1len, s2len) > size_max
> + || MAX (s1len, s2len) > size_max - MIN (s1len, s2len))
> + goto begin_collate;
> +
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.
> if (! __libc_use_alloca ((s1len + s2len) * (sizeof (int32_t) + 1)))
> {
> seq1.idxarr = (int32_t *) malloc ((s1len + s2len) * (sizeof (int32_t) + 1));
> @@ -546,8 +554,10 @@ STRCOLL (const STRING_TYPE *s1, const STRING_TYPE *s2, __locale_t l)
> seq2.rulearr = (unsigned char *) alloca (s2len);
> }
>
> - int rule = 0;
> + int rule;
>
> + begin_collate:
> + rule = 0;