[PATCH libreoffice-4-0] resolved rhbz#918168 ICU: CVE-2013-0900 race condition allow...

Eike Rathke (via Code Review) gerrit at gerrit.libreoffice.org
Wed Mar 6 16:50:50 PST 2013


Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/2576

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/76/2576/1

resolved rhbz#918168 ICU: CVE-2013-0900 race condition allows DoS

Upstream changes
https://ssl.icu-project.org/trac/changeset/32865
https://ssl.icu-project.org/trac/changeset/32908

See also
https://bugzilla.redhat.com/show_bug.cgi?id=918167

Plus changed UMutex to UMTX to make these compile in icu49.

Change-Id: Ida28f7c955959565a2133242b826bb5038758407
---
A icu/icu4c-CVE-2013-0900_umutex.patch
A icu/icu4c.9737.CVE-2013-0900_changesets_32865_32908.patch
M icu/makefile.mk
3 files changed, 305 insertions(+), 0 deletions(-)



diff --git a/icu/icu4c-CVE-2013-0900_umutex.patch b/icu/icu4c-CVE-2013-0900_umutex.patch
new file mode 100644
index 0000000..a705df2
--- /dev/null
+++ b/icu/icu4c-CVE-2013-0900_umutex.patch
@@ -0,0 +1,12 @@
+diff -ruN icu.orig/source/common/locid.cpp icu/source/common/locid.cpp
+--- misc/build/icu.orig/source/common/locid.cpp	2013-03-06 21:27:35.170696967 +0100
++++ misc/build/icu/source/common/locid.cpp	2013-03-06 21:29:47.711378977 +0100
+@@ -53,7 +53,7 @@
+ static Locale *gLocaleCache = NULL;
+ 
+ // gDefaultLocaleMutex protects all access to gDefaultLocalesHashT and gDefaultLocale.
+-static UMutex gDefaultLocaleMutex = U_MUTEX_INITIALIZER;
++static UMTX gDefaultLocaleMutex = NULL;
+ static UHashtable *gDefaultLocalesHashT = NULL;
+ static Locale *gDefaultLocale = NULL;
+ 
diff --git a/icu/icu4c.9737.CVE-2013-0900_changesets_32865_32908.patch b/icu/icu4c.9737.CVE-2013-0900_changesets_32865_32908.patch
new file mode 100644
index 0000000..515e2aa
--- /dev/null
+++ b/icu/icu4c.9737.CVE-2013-0900_changesets_32865_32908.patch
@@ -0,0 +1,291 @@
+Squashed into one:
+https://ssl.icu-project.org/trac/changeset/32865
+https://ssl.icu-project.org/trac/changeset/32908
+
+diff -ruN icu.orig/source/common/locid.cpp icu/source/common/locid.cpp
+--- misc/build/icu.orig/source/common/locid.cpp	2012-04-05 22:46:14.000000000 +0200
++++ misc/build/icu/source/common/locid.cpp	2013-03-05 21:54:11.974608225 +0100
+@@ -1,6 +1,6 @@
+ /*
+  **********************************************************************
+- *   Copyright (C) 1997-2011, International Business Machines
++ *   Copyright (C) 1997-2012, International Business Machines
+  *   Corporation and others.  All Rights Reserved.
+  **********************************************************************
+ *
+@@ -33,6 +33,7 @@
+ #include "unicode/locid.h"
+ #include "unicode/uloc.h"
+ #include "putilimp.h"
++#include "mutex.h"
+ #include "umutex.h"
+ #include "uassert.h"
+ #include "cmemory.h"
+@@ -43,6 +44,21 @@
+ 
+ #define LENGTHOF(array) (int32_t)(sizeof(array)/sizeof((array)[0]))
+ 
++U_CDECL_BEGIN
++static UBool U_CALLCONV locale_cleanup(void);
++U_CDECL_END
++
++U_NAMESPACE_BEGIN
++
++static Locale *gLocaleCache = NULL;
++
++// gDefaultLocaleMutex protects all access to gDefaultLocalesHashT and gDefaultLocale.
++static UMutex gDefaultLocaleMutex = U_MUTEX_INITIALIZER;
++static UHashtable *gDefaultLocalesHashT = NULL;
++static Locale *gDefaultLocale = NULL;
++
++U_NAMESPACE_END
++
+ typedef enum ELocalePos {
+     eENGLISH,
+     eFRENCH,
+@@ -77,10 +93,6 @@
+             UBool valuesToo,
+             UErrorCode *status);
+ 
+-static icu::Locale *gLocaleCache = NULL;
+-static icu::Locale *gDefaultLocale = NULL;
+-static UHashtable *gDefaultLocalesHashT = NULL;
+-
+ U_CDECL_BEGIN
+ //
+ // Deleter function for Locales owned by the default Locale hash table/
+@@ -102,24 +114,19 @@
+     if (gDefaultLocalesHashT) {
+         uhash_close(gDefaultLocalesHashT);   // Automatically deletes all elements, using deleter func.
+         gDefaultLocalesHashT = NULL;
++        gDefaultLocale = NULL;
+     }
+-    else if (gDefaultLocale) {
+-        // The cache wasn't created, and only one default locale was created.
+-        delete gDefaultLocale;
+-    }
+-    gDefaultLocale = NULL;
+ 
+     return TRUE;
+ }
+ U_CDECL_END
+ 
+ U_NAMESPACE_BEGIN
+-//
+-//  locale_set_default_internal.
+-//
+-void locale_set_default_internal(const char *id)
+-{
+-    UErrorCode   status = U_ZERO_ERROR;
++
++Locale *locale_set_default_internal(const char *id, UErrorCode& status) {
++    // Synchronize this entire function.
++    Mutex lock(&gDefaultLocaleMutex);
++    
+     UBool canonicalize = FALSE;
+ 
+     // If given a NULL string for the locale id, grab the default
+@@ -127,17 +134,10 @@
+     //   (Different from most other locale APIs, where a null name means use
+     //    the current ICU default locale.)
+     if (id == NULL) {
+-        umtx_lock(NULL);
+-        id = uprv_getDefaultLocaleID();
+-        umtx_unlock(NULL);
++        id = uprv_getDefaultLocaleID();   // This function not thread safe? TODO: verify.
+         canonicalize = TRUE; // always canonicalize host ID
+     }
+ 
+-    // put the locale id into a canonical form,
+-    //   in preparation for looking up this locale in the hash table of
+-    //   already-created locale objects.
+-    //
+-    status = U_ZERO_ERROR;
+     char localeNameBuf[512];
+ 
+     if (canonicalize) {
+@@ -148,100 +148,37 @@
+     localeNameBuf[sizeof(localeNameBuf)-1] = 0;  // Force null termination in event of
+                                                  //   a long name filling the buffer.
+                                                  //   (long names are truncated.)
++                                                 //
++    if (U_FAILURE(status)) {
++        return gDefaultLocale;
++    }
+ 
+-    // Lazy creation of the hash table itself, if needed.
+-    UBool isOnlyLocale;
+-    UMTX_CHECK(NULL, (gDefaultLocale == NULL), isOnlyLocale);
+-    if (isOnlyLocale) {
+-        // We haven't seen this locale id before.
+-        // Create a new Locale object for it.
+-        Locale *newFirstDefault = new Locale(Locale::eBOGUS);
+-        if (newFirstDefault == NULL) {
+-            // No way to report errors from here.
+-            return;
+-        }
+-        newFirstDefault->init(localeNameBuf, FALSE);
+-        umtx_lock(NULL);
+-        if (gDefaultLocale == NULL) {
+-            gDefaultLocale = newFirstDefault;  // Assignment to gDefaultLocale must happen inside mutex
+-            newFirstDefault = NULL;
+-            ucln_common_registerCleanup(UCLN_COMMON_LOCALE, locale_cleanup);
+-        }
+-        // Else some other thread raced us through here, and set the new Locale.
+-        // Use the hash table next.
+-        umtx_unlock(NULL);
+-        if (newFirstDefault == NULL) {
+-            // We were successful in setting the locale, and we were the first one to set it.
+-            return;
+-        }
+-        // else start using the hash table.
+-    }
+-
+-    // Lazy creation of the hash table itself, if needed.
+-    UBool hashTableNeedsInit;
+-    UMTX_CHECK(NULL, (gDefaultLocalesHashT == NULL), hashTableNeedsInit);
+-    if (hashTableNeedsInit) {
+-        status = U_ZERO_ERROR;
+-        UHashtable *tHashTable = uhash_open(uhash_hashChars, uhash_compareChars, NULL, &status);
++    if (gDefaultLocalesHashT == NULL) {
++        gDefaultLocalesHashT = uhash_open(uhash_hashChars, uhash_compareChars, NULL, &status);
+         if (U_FAILURE(status)) {
+-            return;
++            return gDefaultLocale;
+         }
+-        uhash_setValueDeleter(tHashTable, deleteLocale);
+-        umtx_lock(NULL);
+-        if (gDefaultLocalesHashT == NULL) {
+-            gDefaultLocalesHashT = tHashTable;
+-            ucln_common_registerCleanup(UCLN_COMMON_LOCALE, locale_cleanup);
+-        } else {
+-            uhash_close(tHashTable);
+-            hashTableNeedsInit = FALSE;
+-        }
+-        umtx_unlock(NULL);
++        uhash_setValueDeleter(gDefaultLocalesHashT, deleteLocale);
++        ucln_common_registerCleanup(UCLN_COMMON_LOCALE, locale_cleanup);
+     }
+ 
+-    // Hash table lookup, key is the locale full name
+-    umtx_lock(NULL);
+     Locale *newDefault = (Locale *)uhash_get(gDefaultLocalesHashT, localeNameBuf);
+-    if (newDefault != NULL) {
+-        // We have the requested locale in the hash table already.
+-        // Just set it as default.  Inside the mutex lock, for those troublesome processors.
+-        gDefaultLocale = newDefault;
+-        umtx_unlock(NULL);
+-    } else {
+-        umtx_unlock(NULL);
+-        // We haven't seen this locale id before.
+-        // Create a new Locale object for it.
++    if (newDefault == NULL) {
+         newDefault = new Locale(Locale::eBOGUS);
+         if (newDefault == NULL) {
+-            // No way to report errors from here.
+-            return;
++            status = U_MEMORY_ALLOCATION_ERROR;
++            return gDefaultLocale;
+         }
+         newDefault->init(localeNameBuf, FALSE);
+-
+-        // Add newly created Locale to the hash table of default Locales
+-        const char *key = newDefault->getName();
+-        U_ASSERT(uprv_strcmp(key, localeNameBuf) == 0);
+-        umtx_lock(NULL);
+-        Locale *hashTableVal = (Locale *)uhash_get(gDefaultLocalesHashT, key);
+-        if (hashTableVal == NULL) {
+-            if (hashTableNeedsInit) {
+-                // This is the second request to set the locale.
+-                // Cache the first one.
+-                uhash_put(gDefaultLocalesHashT, (void *)gDefaultLocale->getName(), gDefaultLocale, &status);
+-            }
+-            uhash_put(gDefaultLocalesHashT, (void *)key, newDefault, &status);
+-            gDefaultLocale = newDefault;
+-            // ignore errors from hash table insert.  (Couldn't do anything anyway)
+-            // We can still set the default Locale,
+-            //  it just wont be cached, and will eventually leak.
+-        } else {
+-            // Some other thread raced us through here, and got the new Locale
+-            //   into the hash table before us.  Use that one.
+-            gDefaultLocale = hashTableVal;  // Assignment to gDefaultLocale must happen inside mutex
+-            delete newDefault;
++        uhash_put(gDefaultLocalesHashT, (char*) newDefault->getName(), newDefault, &status);
++        if (U_FAILURE(status)) {
++            return gDefaultLocale;
+         }
+-        umtx_unlock(NULL);
+     }
++    gDefaultLocale = newDefault;
++    return gDefaultLocale;
+ }
++
+ U_NAMESPACE_END
+ 
+ /* sfb 07/21/99 */
+@@ -249,7 +186,8 @@
+ locale_set_default(const char *id)
+ {
+     U_NAMESPACE_USE
+-    locale_set_default_internal(id);
++    UErrorCode status = U_ZERO_ERROR;
++    locale_set_default_internal(id, status);
+ }
+ /* end */
+ 
+@@ -257,7 +195,6 @@
+ locale_get_default(void)
+ {
+     U_NAMESPACE_USE
+-
+     return Locale::getDefault().getName();
+ }
+ 
+@@ -663,19 +600,14 @@
+ const Locale& U_EXPORT2
+ Locale::getDefault()
+ {
+-    const Locale *retLocale;
+-    UMTX_CHECK(NULL, gDefaultLocale, retLocale);
+-    if (retLocale == NULL) {
+-        locale_set_default_internal(NULL);
+-        umtx_lock(NULL);
+-        // Need a mutex  in case some other thread set a new
+-        // default inbetween when we set and when we get the new default.  For
+-        // processors with weak memory coherency, we might not otherwise see all
+-        // of the newly created new default locale.
+-        retLocale = gDefaultLocale;
+-        umtx_unlock(NULL);
++    {
++        Mutex lock(&gDefaultLocaleMutex);
++        if (gDefaultLocale != NULL) {
++            return *gDefaultLocale;
++        }
+     }
+-    return *retLocale;
++    UErrorCode status = U_ZERO_ERROR;
++    return *locale_set_default_internal(NULL, status);
+ }
+ 
+ 
+@@ -692,7 +624,7 @@
+      * This is a convenient way to access the default locale caching mechanisms.
+      */
+     const char *localeID = newLocale.getName();
+-    locale_set_default_internal(localeID);
++    locale_set_default_internal(localeID, status);
+ }
+ 
+ Locale U_EXPORT2
+diff -ruN icu.orig/source/common/unicode/locid.h icu/source/common/unicode/locid.h
+--- misc/build/icu.orig/source/common/unicode/locid.h	2012-04-05 22:46:08.000000000 +0200
++++ misc/build/icu/source/common/unicode/locid.h	2013-03-05 21:54:11.975608225 +0100
+@@ -738,7 +738,7 @@
+      * A friend to allow the default locale to be set by either the C or C++ API.
+      * @internal
+      */
+-    friend void locale_set_default_internal(const char *);
++    friend Locale *locale_set_default_internal(const char *, UErrorCode& status);
+ };
+ 
+ inline UBool
diff --git a/icu/makefile.mk b/icu/makefile.mk
index fbe36fb..98708df 100644
--- a/icu/makefile.mk
+++ b/icu/makefile.mk
@@ -59,6 +59,8 @@
     icu4c-solarisgcc.patch \
     icu4c-mkdir.patch \
     icu4c-buffer-overflow.patch \
+	icu4c.9737.CVE-2013-0900_changesets_32865_32908.patch \
+	icu4c-CVE-2013-0900_umutex.patch \
 
 .IF "$(OS)"=="ANDROID"
 PATCH_FILES+=\

-- 
To view, visit https://gerrit.libreoffice.org/2576
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ida28f7c955959565a2133242b826bb5038758407
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Eike Rathke <erack at redhat.com>



More information about the LibreOffice mailing list