loadarchive: guard against locale-archive corruption
Commit Message
From 1c9ebf0296d4dfe32dd856fc6d1932212a0b0175 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Fri, 18 Oct 2019 17:15:52 -0400
Subject: loadarchive: guard against locale-archive corruption
_nl_load_locale_from_archive() checks for a zero size, but
divides by both (size) and (size-2). Extend the check to
guard against a size of two or less.
Tested by manually corrupting locale-archive and running a program
that calls setlocale() with LOCPATH unset (size is typically very
large).
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1470124
Comments
On 10/18/19 5:21 PM, DJ Delorie wrote:
> From 1c9ebf0296d4dfe32dd856fc6d1932212a0b0175 Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Fri, 18 Oct 2019 17:15:52 -0400
> Subject: loadarchive: guard against locale-archive corruption
>
This is a user visible bug and so needs a sourceware bug.
The sourceware bug should be part of the first git commit message line e.g.
loadarchive: guard against locale-archive corruption (Bug XXXX)
OK for master with bug created and bug # on commit line.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> _nl_load_locale_from_archive() checks for a zero size, but
> divides by both (size) and (size-2). Extend the check to
> guard against a size of two or less.
OK.
>
> Tested by manually corrupting locale-archive and running a program
> that calls setlocale() with LOCPATH unset (size is typically very
> large).
OK.
>
> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1470124
>
> diff --git a/locale/loadarchive.c b/locale/loadarchive.c
> index 981f68d410..b4a73d5c94 100644
> --- a/locale/loadarchive.c
> +++ b/locale/loadarchive.c
> @@ -274,7 +274,7 @@ _nl_load_locale_from_archive (int category, const char **namep)
> + head->namehash_offset);
>
> /* Avoid division by 0 if the file is corrupted. */
> - if (__glibc_unlikely (head->namehash_size == 0))
> + if (__glibc_unlikely (head->namehash_size <= 2))
OK. Reviewed the surrounding code which does the two divisions, and this looks correct.
> goto close_and_out;
>
> idx = hval % head->namehash_size;
>
@@ -274,7 +274,7 @@ _nl_load_locale_from_archive (int category, const char **namep)
+ head->namehash_offset);
/* Avoid division by 0 if the file is corrupted. */
- if (__glibc_unlikely (head->namehash_size == 0))
+ if (__glibc_unlikely (head->namehash_size <= 2))
goto close_and_out;
idx = hval % head->namehash_size;