Bug 681681 (CVE-2010-4756) - CVE-2010-4756 glibc: glob implementation can cause excessive CPU and memory consumption due to crafted glob expressions
Summary: CVE-2010-4756 glibc: glob implementation can cause excessive CPU and memory c...
Keywords:
Status: CLOSED WONTFIX
Alias: CVE-2010-4756
Product: Security Response
Classification: Other
Component: vulnerability
Version: unspecified
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Red Hat Product Security
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-03-02 22:10 UTC by Vincent Danen
Modified: 2019-09-29 12:43 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-15 14:23:42 UTC


Attachments (Terms of Use)
glob_limit (9.36 KB, patch)
2011-05-30 06:48 UTC, Maksymilian
no flags Details | Diff
glob.h.diff (1.06 KB, patch)
2011-05-30 06:49 UTC, Maksymilian
no flags Details | Diff

Description Vincent Danen 2011-03-02 22:10:49 UTC
Common Vulnerabilities and Exposures assigned an identifier CVE-2010-4756 to
the following vulnerability:

Name: CVE-2010-4756
URL: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-4756
Assigned: 20110302
Reference: http://securityreason.com/achievement_securityalert/89
Reference: http://cxib.net/stuff/glob-0day.c
Reference: http://securityreason.com/exploitalert/9223

The glob implementation in the GNU C Library (aka glibc or libc6)
allows remote authenticated users to cause a denial of service (CPU
and memory consumption) via crafted glob expressions that do not match
any pathnames, as demonstrated by glob expressions in STAT commands to
an FTP daemon, a different vulnerability than CVE-2010-2632.

Comment 1 Jakub Jelinek 2011-03-02 22:35:47 UTC
I fail to see why this is considered to be a bug in glibc.  POSIX has clear definition of glob, and glibc just implements it.
It is a bug in network facing services if they pass in unsanitized inputs to glob, without using appropriate rlimits on memory usage or similar.

Comment 2 Vincent Danen 2011-03-03 20:49:42 UTC
(In reply to comment #1)
> I fail to see why this is considered to be a bug in glibc.  POSIX has clear
> definition of glob, and glibc just implements it.
> It is a bug in network facing services if they pass in unsanitized inputs to
> glob, without using appropriate rlimits on memory usage or similar.

I was more or less wondering the same thing but when they assign a CVE name to something we ship, we need to note it.  I don't believe this should be a problem with glibc itself (although perhaps there is a mechanism that could be used to make glibc safer in this regard, I don't know; I've not looked at this too closely).

This one seemed fuzzy to me, and I think it was only noted because a reporter's advisory (securityreason.com) noted libc and glibc.  They also assigned CVEs to sftp and vsftpd for globbing issues like this.

Having said that, if we strongly feel that this is something that cannot or should not be fixed in glibc, then we need to be able to have something to point customers to when they ask about it, hence the bug.

Comment 4 Tomas Hoger 2011-03-14 15:22:31 UTC
Jakub, do you remember any upstream discussion regarding possible addition of GLOB_LIMIT flag as implemented on various BSD systems:

  GLOB_LIMIT       Limit the amount of memory used to store matched strings
                   to 64K, the number of stat(2) calls to 128, and the
                   number of readdir(3) calls to 16K.  This option should
                   be set for programs that can be coerced to a denial of
                   service attack via patterns that expand to a very large
                   number of matches, such as a long string of `*/../*/..'.

glibc glob() already has couple of other non-POSIX extensions.

Comment 5 Jakub Jelinek 2011-03-14 16:03:36 UTC
I don't remember this would be ever discussed.
I don't think it is very nice option, because it hardcodes all the limits, much nicer would be to be able to say explicitly what limits you want to pose, perhaps
have a flag which causes errfunc to be called with 0 errnum even in the non-error cases so that the app can decide when to stop?

Anyway, this needs to be discussed upstream first.

Comment 6 Tomas Hoger 2011-03-14 16:26:54 UTC
(In reply to comment #5)
> I don't remember this would be ever discussed.

Thanks!

> I don't think it is very nice option, because it hardcodes all the limits,
> much nicer would be to be able to say explicitly what limits you want to pose,
> perhaps have a flag which causes errfunc to be called with 0 errnum even in
> the non-error cases so that the app can decide when to stop?

Sounds like GLOB_ALTDIRFUNC may already be a way to enforce most of the limits set by GLOB_LIMIT.

Comment 8 Tomas Hoger 2011-03-15 14:23:42 UTC
Closing this based on the previous discussion.  As mentioned, there's currently no plan to enforce arbitrary limits in glob() by default.

Comment 9 Maksymilian 2011-04-30 14:49:12 UTC
glob gnu libc, add GLOB_LIMIT to fix CVE-2010-4756

reason: to protect software like inetutils-ftpd and others. See code of inetutils-ftpd

./ftpd/ftpcmd.c:				if (glob((yyvsp[(1) - (1)].s), flags, NULL, &gl) ||
./ftpd/ftpcmd.y:				if (glob($1, flags, NULL, &gl) ||

flags

./ftpd/ftpcmd.c: 				int flags = GLOB_NOCHECK;
./ftpd/ftpcmd.c:				flags |= GLOB_BRACE;
./ftpd/ftpcmd.c:				flags |= GLOB_QUOTE;
./ftpd/ftpcmd.c:				flags |= GLOB_TILDE;
./ftpd/ftpcmd.c:				if (glob((yyvsp[(1) - (1)].s), flags, NULL, &gl) ||
./ftpd/ftpcmd.y:				int flags = GLOB_NOCHECK;
./ftpd/ftpcmd.y:				flags |= GLOB_BRACE;
./ftpd/ftpcmd.y:				flags |= GLOB_QUOTE;
./ftpd/ftpcmd.y:				flags |= GLOB_TILDE;
./ftpd/ftpcmd.y:				if (glob($1, flags, NULL, &gl) ||
./ftpd/ftpd.c:      int flags = GLOB_NOCHECK;
./ftpd/ftpd.c:      flags |= GLOB_BRACE;
./ftpd/ftpd.c:      flags |= GLOB_QUOTE;
./ftpd/ftpd.c:      flags |= GLOB_TILDE;
./ftpd/ftpd.c:      if (glob (whichf, flags, 0, &gl))

glob(3) used in ftpd without GLOB_LIMIT, may provide to DoS, see example for openssh CVE-2010-4755

To add GLOB_LIMIT in to gnu libc, first we need define new flag in glob.h

------------
#define	GLOB_PERIOD	(1 << 7)/* Leading `.' can be matched by metachars.  */
+ #define GLOB_LIMIT (1 << 15)/* glob limit */

#if !defined __USE_POSIX2 || defined __USE_BSD || defined __USE_GNU
...
# define GLOB_ONLYDIR	 (1 << 13)/* Match only directories.  */
# define GLOB_TILDE_CHECK (1 << 14)/* Like GLOB_TILDE but return an error
				      if the user name is not available.  */

# define __GLOB_FLAGS	(GLOB_ERR|GLOB_MARK|GLOB_NOSORT|GLOB_DOOFFS| \
			 GLOB_NOESCAPE|GLOB_NOCHECK|GLOB_APPEND|     \
			 GLOB_PERIOD|GLOB_ALTDIRFUNC|GLOB_BRACE|     \
+			 GLOB_NOMAGIC|GLOB_TILDE|GLOB_ONLYDIR|GLOB_TILDE_CHECK|GLOB_LIMIT)
#else
# define __GLOB_FLAGS	(GLOB_ERR|GLOB_MARK|GLOB_NOSORT|GLOB_DOOFFS| \
			 GLOB_NOESCAPE|GLOB_NOCHECK|GLOB_APPEND|     \
+			 GLOB_PERIOD|GLOB_LIMIT)
#endif
------------

then add constans limits in to glob.c

------------
#ifndef attribute_hidden
# define attribute_hidden
#endif

+ #define	GLOB_LIMIT_STAT		256	/* number of stat system calls */
+ #define	GLOB_LIMIT_READDIR	16384	/* total readdir calls */
+ #define	GLOB_LIMIT_RECURSION	128	/* number of recursion glob */
+ #define	GLOB_LIMIT_STRING	65536	/* total malloc/alloca */
+ #define	GLOB_LIMIT_PATH		1024	/* number of path elements */
+
+ /* 
+ * NOTE:
+ * limit[0] stat
+ * limit[1] readdir
+ * limit[2] recursion
+ * limit[3] string
+ */
+ int limit[4] = { 0, 0, 0, 0 };

static int glob_in_dir (const char *pattern, const char *directory,
			int flags, int (*errfunc) (const char *, int),
------------

Let`s starts add limits in to glob(3). First STAT and READDIR.

------------
  glob_t dirs;

+  if ((flags & GLOB_LIMIT) && limit[0] >= GLOB_LIMIT_STAT) return GLOB_ABORTED;
+  if ((flags & GLOB_LIMIT) && limit[1] >= GLOB_LIMIT_READDIR) return GLOB_ABORTED;
+  if ((flags & GLOB_LIMIT) && limit[2] >= GLOB_LIMIT_RECURSION) return GLOB_ABORTED;
  
  if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
------------

------------
  /* Now test whether we looked for "~" or "~NAME".  In this case we
     can give the answer now.  */
  if (filename == NULL)
    {
+	if ((flags & GLOB_LIMIT) && limit[0] >= GLOB_LIMIT_STAT) return GLOB_NOSPACE;
+	else if(flags & GLOB_LIMIT) limit[0]++;

      struct stat st;
      struct_stat64 st64;
------------

------------
  if (flags & GLOB_MARK)
    {
+	if ((flags & GLOB_LIMIT) && limit[0] >= GLOB_LIMIT_STAT) return GLOB_NOSPACE;
+	else if(flags & GLOB_LIMIT) limit[0]++;

      /* Append slashes to directory names.  */
------------

------------
  mempcpy (mempcpy (mempcpy (fullname, dir, dirlen), "/", 1),
	   fname, fnamelen + 1);
+	if ((flags & GLOB_LIMIT) && limit[0] >= GLOB_LIMIT_STAT) return GLOB_NOSPACE;
+	else if(flags & GLOB_LIMIT) limit[0]++;

# ifdef _LIBC
------------

------------
  init_names.count = INITIAL_COUNT;

+	if ((flags & GLOB_LIMIT) && limit[0] >= GLOB_LIMIT_STAT) return GLOB_NOSPACE;
+	else if(flags & GLOB_LIMIT) limit[0]++;
  
  meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
------------

------------
	      const char *name;
	      size_t len;
+      	      if ((flags & GLOB_LIMIT) && limit[1] >= GLOB_LIMIT_READDIR) return GLOB_NOSPACE;
+	      else if(flags & GLOB_LIMIT) limit[1]++;
#if defined _LIBC && !defined COMPILE_GLOB64
------------

We have defined places, where readdir and stat are used. Now is the time to reduce recursion simlar to recursion in vsftpd 2.3.2.
IMPORTANT: please verifity again, that all dangerous glob(3) calls are limited.

------------
	  p = begin + 1;
	  while (1)
	    {

+		if ((flags & GLOB_LIMIT) && limit[2] >= GLOB_LIMIT_RECURSION) return GLOB_NOSPACE;
+		else if(flags & GLOB_LIMIT) limit[2]++;

	      int result;
------------

------------
	  dirs.gl_lstat = pglob->gl_lstat;
	}
+		if ((flags & GLOB_LIMIT) && limit[2] >= GLOB_LIMIT_RECURSION) return GLOB_NOSPACE;
+		else if(flags & GLOB_LIMIT) limit[2]++;

		status = glob (dirname,
		     ((flags & (GLOB_ERR | GLOB_NOESCAPE
				| GLOB_ALTDIRFUNC))
      status = xxxglob (dirname,
------------

and change flags (add GLOB_LIMIT)

------------
from:
		status = glob (dirname,
		     ((flags & (GLOB_ERR | GLOB_NOESCAPE
				| GLOB_ALTDIRFUNC))

to:
      status = xxxglob (dirname,
		     ((flags & (GLOB_ERR | GLOB_NOESCAPE
				| GLOB_ALTDIRFUNC | GLOB_LIMIT))
------------

------------
from:
static int prefix_array (const char *prefix, char **array, size_t n) __THROW;

to:
static int prefix_array (const char *prefix, char **array, size_t n, int flags) __THROW;
------------

now prefix_array calls

------------
from:
	  /* Stick the directory on the front of each name.  */
	  if (prefix_array (dirs.gl_pathv[i],
			    &pglob->gl_pathv[old_pathc + pglob->gl_offs],
			    pglob->gl_pathc - old_pathc))
	    {
	      globfree (&dirs);

to:

	  /* Stick the directory on the front of each name.  */
	  if (prefix_array (dirs.gl_pathv[i],
			    &pglob->gl_pathv[old_pathc + pglob->gl_offs],
			    pglob->gl_pathc - old_pathc,flags))
	    {
	      globfree (&dirs);
------------

------------
from:
	  /* Stick the directory on the front of each name.  */
	  if (prefix_array (dirname,
			    &pglob->gl_pathv[old_pathc + pglob->gl_offs],
			    pglob->gl_pathc - old_pathc))
	    {
	      globfree (pglob);
	      pglob->gl_pathc = 0;

to:

	  /* Stick the directory on the front of each name.  */
	  if (prefix_array (dirname,
			    &pglob->gl_pathv[old_pathc + pglob->gl_offs],
			    pglob->gl_pathc - old_pathc,flags))
	    {
	      globfree (pglob);
	      pglob->gl_pathc = 0;
------------

Now we should control all malloc,alloca calls to reduce memory exhaustion. Anyway, it may be difficult and these fix define wrong malloc. Also we should change calls like

------------
#ifdef __GNUC__
	  char onealt[strlen (pattern) - 1];
#else
------------

Example of use GLOB_LIMIT_STRING
------------
      result = 0;

+      if ((flags & GLOB_LIMIT) && (pglob->gl_pathc + pglob->gl_offs + nfound + 1)+limit[3] >= GLOB_LIMIT_STRING) return GLOB_NOSPACE;
+      else if((flags & GLOB_LIMIT)) limit[3]+= (pglob->gl_pathc + pglob->gl_offs + nfound + 1);

      char **new_gl_pathv;
      new_gl_pathv
------------

Example of use GLOB_LIMIT_PATH
------------
	  const char *rest;
	  size_t rest_len;
	  
	if ((flags & GLOB_LIMIT) && strlen(pattern) >= GLOB_LIMIT_PATH) return GLOB_NOSPACE;
	  
#ifdef __GNUC__
	  char onealt[strlen (pattern) - 1];
------------

Comment 10 Tomas Hoger 2011-05-09 16:51:31 UTC
(In reply to comment #9)
> glob gnu libc, add GLOB_LIMIT to fix CVE-2010-4756
> 
> reason: to protect software like inetutils-ftpd and others. See code of
> inetutils-ftpd

Even with GLOB_LIMIT supported by glibc glob, such ftpd daemons would need to be modified to start using it.

> To add GLOB_LIMIT in to gnu libc, first we need define new flag in glob.h

If you have a proposed fix to add GLOB_LIMIT support to glibc glob, would you mind filing an upstream bug for it and attach it as unified diff, rather than a do-this-and-that-manually diff.

Comment 11 Maksymilian 2011-05-30 06:48:51 UTC
Created attachment 501710 [details]
glob_limit

GLOB_LIMIT support for glibc 2.13

LIMITS:
struct glob_limit {
	size_t l_stat; // stat calls
	size_t l_readdir; // readdir calls
	size_t l_recursion; // recursion
};

I have not changed stack calls because is a global problem in glibc. Anyway l_recursion should solve memory exhaustion problem.

Compare this pattern with GLOB_LIMIT:
{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*/{..,..,..}/*sds

best

Comment 12 Maksymilian 2011-05-30 06:49:34 UTC
Created attachment 501711 [details]
glob.h.diff


Note You need to log in before you can comment on or make changes to this bug.