Skip to content

Commit

Permalink
libintl: Fix pointer use after free and make error handling robuster
Browse files Browse the repository at this point in the history
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>.
  • Loading branch information
codonell authored and ueno committed Sep 6, 2013
1 parent 70c796d commit e3e7f52
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 7 deletions.
13 changes: 13 additions & 0 deletions 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
Expand Down
27 changes: 21 additions & 6 deletions gettext-runtime/intl/dcigettext.c
Expand Up @@ -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];
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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))
{
Expand All @@ -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
Expand Down
9 changes: 8 additions & 1 deletion gettext-runtime/intl/loadmsgcat.c
Expand Up @@ -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)
Expand All @@ -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:
Expand Down

0 comments on commit e3e7f52

Please sign in to comment.