Re: Possible DoS issues in gettext

[Bruno Haible]

Hello,

Ismail Dönmez wrote:
> Multiple DoS issues are reported against PHP's gettext functions [0] but 
> looks 
> like its a gettext problem. Is it possible to fix these problems in gettext 
> itself?
> 
> [0] http://www.securityfocus.com/archive/1/483648/30/30/threaded

Thank you for reporting this. I have forwarded it as a glibc bug at
  <http://sourceware.org/bugzilla/show_bug.cgi?id=5346>

Applying the fix also to GNU gettext as follows.


2007-11-15  Bruno Haible  <address@hidden>

        Avoid crash by stack overflow when msgid is very long.
        * dcigettext.c (struct known_translation_t): Turn msgid into a union.
        (transcmp): Use the appropriate part of s1->msgid and s2->msgid.
        (DCIGETTEXT): Change the allocation of the 'search' variable so that
        it needs only fixed stack space. Delay the initialization of
        msgid_len until it is needed.
        Reported by Laurent Gaffié <address@hidden> via
        Ismail Dönmez <address@hidden>.

*** gettext-runtime/intl/dcigettext.c.bak       21 Oct 2007 18:42:41 -0000      
1.38
--- gettext-runtime/intl/dcigettext.c   16 Nov 2007 00:58:59 -0000
***************
*** 269,275 ****
    size_t translation_length;
  
    /* Pointer to the string in question.  */
!   char msgid[ZERO];
  };
  
  gl_rwlock_define_initialized (static, tree_lock)
--- 269,280 ----
    size_t translation_length;
  
    /* Pointer to the string in question.  */
!   union
!     {
!       char appended[ZERO];  /* used if domain != NULL */
!       const char *ptr;      /* used if domain == NULL */
!     }
!   msgid;
  };
  
  gl_rwlock_define_initialized (static, tree_lock)
***************
*** 288,294 ****
    s1 = (const struct known_translation_t *) p1;
    s2 = (const struct known_translation_t *) p2;
  
!   result = strcmp (s1->msgid, s2->msgid);
    if (result == 0)
      {
        result = strcmp (s1->domainname, s2->domainname);
--- 293,300 ----
    s1 = (const struct known_translation_t *) p1;
    s2 = (const struct known_translation_t *) p2;
  
!   result = strcmp (s1->domain != NULL ? s1->msgid.appended : s1->msgid.ptr,
!                  s2->domain != NULL ? s2->msgid.appended : s2->msgid.ptr);
    if (result == 0)
      {
        result = strcmp (s1->domainname, s2->domainname);
***************
*** 501,509 ****
    char *retval;
    size_t retlen;
    int saved_errno;
!   struct known_translation_t *search;
    struct known_translation_t **foundp = NULL;
-   size_t msgid_len;
  #if defined HAVE_PER_THREAD_LOCALE && !defined IN_LIBGLOCALE
    const char *localename;
  #endif
--- 507,514 ----
    char *retval;
    size_t retlen;
    int saved_errno;
!   struct known_translation_t search;
    struct known_translation_t **foundp = NULL;
  #if defined HAVE_PER_THREAD_LOCALE && !defined IN_LIBGLOCALE
    const char *localename;
  #endif
***************
*** 539,553 ****
      category = LC_MESSAGES;
  #endif
  
-   msgid_len = strlen (msgid1) + 1;
- 
    /* Try to find the translation among those which we found at
       some time.  */
!   search = (struct known_translation_t *)
!          alloca (offsetof (struct known_translation_t, msgid) + msgid_len);
!   memcpy (search->msgid, msgid1, msgid_len);
!   search->domainname = domainname;
!   search->category = category;
  #ifdef HAVE_PER_THREAD_LOCALE
  # ifndef IN_LIBGLOCALE
  #  ifdef _LIBC
--- 544,555 ----
      category = LC_MESSAGES;
  #endif
  
    /* Try to find the translation among those which we found at
       some time.  */
!   search.domain = NULL;
!   search.msgid.ptr = msgid1;
!   search.domainname = domainname;
!   search.category = category;
  #ifdef HAVE_PER_THREAD_LOCALE
  # ifndef IN_LIBGLOCALE
  #  ifdef _LIBC
***************
*** 571,590 ****
  #   endif
  #  endif
  # endif
!   search->localename = localename;
  # ifdef IN_LIBGLOCALE
!   search->encoding = encoding;
  # endif
  
    /* Since tfind/tsearch manage a balanced tree, concurrent tfind and
       tsearch calls can be fatal.  */
    gl_rwlock_rdlock (tree_lock);
  
!   foundp = (struct known_translation_t **) tfind (search, &root, transcmp);
  
    gl_rwlock_unlock (tree_lock);
  
-   freea (search);
    if (foundp != NULL && (*foundp)->counter == _nl_msg_cat_cntr)
      {
        /* Now deal with plural.  */
--- 573,591 ----
  #   endif
  #  endif
  # endif
!   search.localename = localename;
  # ifdef IN_LIBGLOCALE
!   search.encoding = encoding;
  # endif
  
    /* Since tfind/tsearch manage a balanced tree, concurrent tfind and
       tsearch calls can be fatal.  */
    gl_rwlock_rdlock (tree_lock);
  
!   foundp = (struct known_translation_t **) tfind (&search, &root, transcmp);
  
    gl_rwlock_unlock (tree_lock);
  
    if (foundp != NULL && (*foundp)->counter == _nl_msg_cat_cntr)
      {
        /* Now deal with plural.  */
***************
*** 773,781 ****
--- 774,784 ----
              if (foundp == NULL)
                {
                  /* Create a new entry and add it to the search tree.  */
+                 size_t msgid_len;
                  size_t size;
                  struct known_translation_t *newp;
  
+                 msgid_len = strlen (msgid1) + 1;
                  size = offsetof (struct known_translation_t, msgid)
                         + msgid_len + domainname_len + 1;
  #ifdef HAVE_PER_THREAD_LOCALE
***************
*** 790,796 ****
  #endif
  
                      new_domainname =
!                       (char *) mempcpy (newp->msgid, msgid1, msgid_len);
                      memcpy (new_domainname, domainname, domainname_len + 1);
  #ifdef HAVE_PER_THREAD_LOCALE
                      new_localename = new_domainname + domainname_len + 1;
--- 793,800 ----
  #endif
  
                      new_domainname =
!                       (char *) mempcpy (newp->msgid.appended, msgid1,
!                                         msgid_len);
                      memcpy (new_domainname, domainname, domainname_len + 1);
  #ifdef HAVE_PER_THREAD_LOCALE
                      new_localename = new_domainname + domainname_len + 1;