Patchwork loadarchive: guard against locale-archive corruption

login
register
mail settings
Submitter DJ Delorie
Date Oct. 18, 2019, 9:21 p.m.
Message ID <xn1rv9hhqq.fsf@greed.delorie.com>
Download mbox | patch
Permalink /patch/35149/
State New
Headers show

Comments

DJ Delorie - Oct. 18, 2019, 9:21 p.m.
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
Carlos O'Donell - Oct. 18, 2019, 9:29 p.m.
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;
>

Patch

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))
     goto close_and_out;
 
   idx = hval % head->namehash_size;