Bug 2459 - Use of uninitialised variable in fnmatch when matching long strings
Summary: Use of uninitialised variable in fnmatch when matching long strings
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.3.6
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-14 14:58 UTC by Richard Smith
Modified: 2018-04-19 13:45 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Suggested fix for bug (273 bytes, patch)
2006-03-14 14:59 UTC, Richard Smith
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Smith 2006-03-14 14:58:01 UTC
In fnmatch() in posix/fnmatch.c, the variable wstring is left uninitialised if
strlen(string) >= 1024.  This appears to the cause of segfaults in GNU ld when
heavily templated C++ results in very long ELF section names.
Comment 1 Richard Smith 2006-03-14 14:59:26 UTC
Created attachment 918 [details]
Suggested fix for bug

The attached patch fixes it by initialising wstring in the same way as
wpattern.
Comment 2 Jakub Jelinek 2006-03-14 15:05:15 UTC
I think you first need to answer what libc you are patching, because there
is no such code in glibc 2.4 nor glibc 2.3.6.
Comment 3 Richard Smith 2006-03-14 15:15:55 UTC
It's against the code added in CVS version 1.50.4.1 on the glibc-2_3-branch:

2005-03-29  Jakub Jelinek  <jakub@redhat.com>

	[BZ #1087]
	* posix/fnmatch.c (fnmatch): For short patterns or strings attempt to
	avoid calling mbsrtowcs twice.

or the equivalent code in 1.51 on CVS head.

(I'm actually using a version of 2.3.6 with a significant set of additional
vendor patches.)
Comment 4 Jakub Jelinek 2006-03-14 15:23:34 UTC
glibc-2_3-branch has:
#ifdef _LIBC
      n = strnlen (string, 1024);
#else
      n = strlen (string);
#endif
      p = string;
      if (__builtin_expect (n < 1024, 1))
        {
          wstring = (wchar_t *) alloca ((n + 1) * sizeof (wchar_t));
          n = mbsrtowcs (wstring, &p, n + 1, &ps);
          if (__builtin_expect (n == (size_t) -1, 0))
            /* Something wrong.
               XXX Do we have to set `errno' to something which mbsrtows hasn't
               already done?  */
            return -1;
          if (p)
            memset (&ps, '\0', sizeof (ps));
        }
      if (__builtin_expect (p != NULL, 0))
        {
          n = mbsrtowcs (NULL, &string, 0, &ps);
          if (__builtin_expect (n == (size_t) -1, 0))
            /* Something wrong.
               XXX Do we have to set `errno' to something which mbsrtows hasn't
               already done?  */
            return -1;
          wstring = (wchar_t *) alloca ((n + 1) * sizeof (wchar_t));
          assert (mbsinit (&ps));
          (void) mbsrtowcs (wstring, &string, n + 1, &ps);
        }

      return internal_fnwmatch (wpattern, wstring, wstring + n,
                                flags & FNM_PERIOD, flags);

No try_singlebyte label and I don't see how can wstring end up being
uninitialized (well, GCC will warn, but that's GCC deficiency).
Comment 5 Richard Smith 2006-03-14 15:41:14 UTC
*Embarassed pause*

Sorry about that -- I should have checked more carefully precisely what my
distribution (Gentoo) was doing to that file.  Having found one patch to that
function, I failed to notice a second one that introduces this bug by deleting
the line

  wstring = (wchar_t *) alloca ((n + 1) * sizeof (wchar_t));

Anyway, this is clearly Gentoo's bug not yours.  Sorry to have inconvenienced
you with it.
Comment 6 Andreas Jaeger 2006-03-15 08:49:58 UTC
Ok, let's close since this is a Gentoo bug.