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.

(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.

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.

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.

(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.

Closing this based on the previous discussion.  As mentioned, there's currently no plan to enforce arbitrary limits in glob() by default.

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];
------------

(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.

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

Created attachment 501711 [details]
glob.h.diff