libintl: Fix pointer use after free and make error handling robuster

Previously, _nl_find_msg had a use of pointer after free'd.  Also,
some callers of the function didn't check the failure.

Merge changes from glibc.  Reported by Carlos O'Donell in
<https://savannah.gnu.org/bugs/?38930>.

diff --git a/gettext-runtime/intl/ChangeLog b/gettext-runtime/intl/ChangeLog
index 562c282ac9525951fa9f6e1c35f7162d4e90fd63..53168881e6d021b677814f717d62c9a96ab1ffac 100644 (file)
--- a/gettext-runtime/intl/ChangeLog
+++ b/gettext-runtime/intl/ChangeLog
@@ -1,3 +1,16 @@
+2013-05-07  Carlos O'Donell  <carlos@redhat.com>
+            Jeff Law  <law@redhat.com>
+
+       * dcigettext.c (DCIGETTEXT): Skip translating if _nl_find_msg
+       returns -1.
+       (_nl_find_msg): Return -1 if recursive call returned -1. If
+       newmem is null return -1.
+       * loadmsgcat.c (_nl_load_domain): If _nl_find_msg returns -1
+       abort loading the domain.
+
+       * dcigettext.c (_nl_find_msg): Avoid use after potential
+       free.  Simplify list management for _LIBC case.
+
 2013-03-07  Daiki Ueno  <ueno@gnu.org>
 
        * setlocale.c (libintl_setlocale): Signal a change of the loaded

diff --git a/gettext-runtime/intl/dcigettext.c b/gettext-runtime/intl/dcigettext.c
index be2dcebedb2d72af5e629ade3395e4e63a60beb5..c55d7a4c82d0b2999ce8303ed98818b974c94802 100644 (file)
--- a/gettext-runtime/intl/dcigettext.c
+++ b/gettext-runtime/intl/dcigettext.c
@@ -743,6 +743,11 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2,
                                         msgid1, 1, &retlen);
 #endif
 
+                 /* Resource problems are not fatal, instead we return no
+                    translation.  */
+                 if (__builtin_expect (retval == (char *) -1, 0))
+                   goto return_untranslated;
+
                  if (retval != NULL)
                    {
                      domain = domain->successor[cnt];
@@ -1103,6 +1108,11 @@ _nl_find_msg (struct loaded_l10nfile *domain_file,
                _nl_find_msg (domain_file, domainbinding, "", 0, &nullentrylen);
 # endif
 
+             /* Resource problems are fatal.  If we continue onwards we will
+                only attempt to calloc a new conv_tab and fail later.  */
+             if (__builtin_expect (nullentry == (char *) -1, 0))
+               return (char *) -1;
+
              if (nullentry != NULL)
                {
                  const char *charsetstr;
@@ -1328,7 +1338,7 @@ _nl_find_msg (struct loaded_l10nfile *domain_file,
                                                             freemem_size);
 # ifdef _LIBC
                      if (newmem != NULL)
-                       transmem_list = transmem_list->next;
+                       transmem_list = newmem;
                      else
                        {
                          struct transmem_list *old = transmem_list;
@@ -1343,6 +1353,16 @@ _nl_find_msg (struct loaded_l10nfile *domain_file,
                      malloc_count = 1;
                      freemem_size = INITIAL_BLOCK_SIZE;
                      newmem = (transmem_block_t *) malloc (freemem_size);
+# ifdef _LIBC
+                     if (newmem != NULL)
+                       {
+                         /* Add the block to the list of blocks we have to free
+                            at some point.  */
+                         newmem->next = transmem_list;
+                         transmem_list = newmem;
+                       }
+                     /* Fall through and return -1.  */
+# endif
                    }
                  if (__builtin_expect (newmem == NULL, 0))
                    {
@@ -1353,11 +1373,6 @@ _nl_find_msg (struct loaded_l10nfile *domain_file,
                    }
 
 # ifdef _LIBC
-                 /* Add the block to the list of blocks we have to free
-                    at some point.  */
-                 newmem->next = transmem_list;
-                 transmem_list = newmem;
-
                  freemem = (unsigned char *) newmem->data;
                  freemem_size -= offsetof (struct transmem_list, data);
 # else

diff --git a/gettext-runtime/intl/loadmsgcat.c b/gettext-runtime/intl/loadmsgcat.c
index 9abee08a9eaa8d763babecaf7dcf5e3acc0a3799..3b8ff18976745fdf2cc682fda8f4fd3d7b519d34 100644 (file)
--- a/gettext-runtime/intl/loadmsgcat.c
+++ b/gettext-runtime/intl/loadmsgcat.c
@@ -1258,7 +1258,7 @@ _nl_load_domain (struct loaded_l10nfile *domain_file,
     default:
       /* This is an invalid revision.  */
     invalid:
-      /* This is an invalid .mo file.  */
+      /* This is an invalid .mo file or we ran out of resources.  */
       free (domain->malloced);
 #ifdef HAVE_MMAP
       if (use_mmap)
@@ -1283,6 +1283,13 @@ _nl_load_domain (struct loaded_l10nfile *domain_file,
 #else
   nullentry = _nl_find_msg (domain_file, domainbinding, "", 0, &nullentrylen);
 #endif
+  if (__builtin_expect (nullentry == (char *) -1, 0))
+    {
+#ifdef _LIBC
+      __libc_rwlock_fini (domain->conversions_lock);
+#endif
+      goto invalid;
+    }
   EXTRACT_PLURAL_EXPRESSION (nullentry, &domain->plural, &domain->nplurals);
 
  out: