Re: [bug-gettext] Gettext 0.19.4 has buggy msgunfmt-3 test?

[Daiki Ueno]

Max Lin <address@hidden> writes:

> On openSUSE Build Service we see gettext 0.19.4 has the random test
> failure in msgunfmt-3. I've trying to debugging it on my local
> machine, my first suspicion is about memory size, it looks like this
> test requires a certain enough number of virtual memory, otherwise it
> will test fail, for example, if I did 'ulimit -v 40000' on my local
> machine then it fails with test, but it does passed if sets
> unlimited. *But* I can not confirm it was memory related, so any
> debugging way/suggestion? or it is a known issue in 0.19.4 actually?

Thanks for reporting that.  In 0.19.4, we added several integer overflow
checks in msgunfmt based on a report from a fuzzying tool:
http://lists.gnu.org/archive/html/bug-gettext/2014-12/msg00005.html
and it was a leftover.  The attached patch should fix this.

It would be nice if we could detect it reliably in msgunfmt-3 test,
though 'ulimit -v' seems not portable.

Regards,
--
Daiki Ueno

>From a4bf41a3705f1e57e0b7c418f16fec1fd11b5252 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Tue, 10 Mar 2015 09:43:41 +0900
Subject: [PATCH] msgunfmt: Check allocated size for static segment

Reported by Max Lin in:
http://lists.gnu.org/archive/html/bug-gettext/2015-03/msg00005.html
* read-mo.c (get_sysdep_string): Check if the embedded segment
size is valid, before adding it to the string length.
---
 gettext-tools/src/ChangeLog | 8 ++++++++
 gettext-tools/src/read-mo.c | 6 ++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/gettext-tools/src/ChangeLog b/gettext-tools/src/ChangeLog
index 754a9dc..423ba5a 100644
--- a/gettext-tools/src/ChangeLog
+++ b/gettext-tools/src/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-10  Daiki Ueno  <address@hidden>
+
+       msgunfmt: Check allocated size for static segment
+       Reported by Max Lin in:
+       http://lists.gnu.org/archive/html/bug-gettext/2015-03/msg00005.html
+       * read-mo.c (get_sysdep_string): Check if the embedded segment
+       size is valid, before adding it to the string length.
+
 2015-03-02  Daiki Ueno  <address@hidden>
 
        xgettext: Support message syntax checks
diff --git a/gettext-tools/src/read-mo.c b/gettext-tools/src/read-mo.c
index b97bbad..aca5a93 100644
--- a/gettext-tools/src/read-mo.c
+++ b/gettext-tools/src/read-mo.c
@@ -146,7 +146,7 @@ get_sysdep_string (const struct binary_mo_file *bfp, size_t 
offset,
   char *string;
   size_t i;
   char *p;
-  nls_uint32 s_offset;
+  nls_uint32 s_offset = get_uint32 (bfp, offset);
 
   /* Compute the length.  */
   length = 0;
@@ -158,8 +158,11 @@ get_sysdep_string (const struct binary_mo_file *bfp, 
size_t offset,
       nls_uint32 ss_length;
       nls_uint32 ss_offset;
       size_t ss_end;
+      size_t s_end = xsum (s_offset, segsize);
       size_t n;
 
+      if (size_overflow_p (s_end) || s_end > bfp->size)
+        error (EXIT_FAILURE, 0, _("file \"%s\" is truncated"), bfp->filename);
       length += segsize;
 
       if (sysdepref == SEGMENTS_END)
@@ -190,7 +193,6 @@ get_sysdep_string (const struct binary_mo_file *bfp, size_t 
offset,
   /* Allocate and fill the string.  */
   string = XNMALLOC (length, char);
   p = string;
-  s_offset = get_uint32 (bfp, offset);
   for (i = 4; ; i += 8)
     {
       nls_uint32 segsize = get_uint32 (bfp, offset + i);
-- 
2.1.3