From 6c3febd040e90250e156dad8f4025ef416f290a5 Mon Sep 17 00:00:00 2001 From: Carlos O'Donell Date: Fri, 6 Sep 2013 15:52:57 +0900 Subject: [PATCH 2/4] 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 . --- gettext-runtime/intl/ChangeLog | 13 +++++++++++++ gettext-runtime/intl/dcigettext.c | 27 +++++++++++++++++++++------ gettext-runtime/intl/loadmsgcat.c | 9 ++++++++- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/gettext-runtime/intl/ChangeLog b/gettext-runtime/intl/ChangeLog index 562c282..5316888 100644 --- a/gettext-runtime/intl/ChangeLog +++ b/gettext-runtime/intl/ChangeLog @@ -1,3 +1,16 @@ +2013-05-07 Carlos O'Donell + Jeff Law + + * 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 * 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 be2dceb..c55d7a4 100644 --- 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 9abee08..3b8ff18 100644 --- 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: -- 1.8.4.3