Date: Sun, 18 Mar 2001 16:00:34 -0600 From: Jonathan Lemon <jlemon@flugsvamp.com> To: Matt Dillon <dillon@earth.backplane.com> Cc: Jonathan Lemon <jlemon@flugsvamp.com>, stable@freebsd.org Subject: Re: Not only ftpd's problem with ls */../*..... Message-ID: <20010318160034.F82645@prism.flugsvamp.com> In-Reply-To: <200103180543.f2I5hb398084@earth.backplane.com> References: <local.mail.freebsd-stable/200103172107.f2HL7Ea02611@cwsys.cwsent.com> <200103172253.f2HMrZ008412@prism.flugsvamp.com> <200103180027.f2I0RSn96769@earth.backplane.com> <20010317222918.B82645@prism.flugsvamp.com> <200103180543.f2I5hb398084@earth.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Mar 17, 2001 at 09:43:37PM -0800, Matt Dillon wrote:
>
> :> be set with a new api call, setgloblimit()? Or perhaps even just
> :> have the setgloblimit() API call and don't even bother with a new flag.
> :
> :I would think a better idea is to have the system default to a hard
> :limit, and then allow those programs that know better to override it.
> :This way, we catch most naive uses of glob, while allowing those users
> :who actually want to iterate over more paths to continue. You'll also
> :note that the glob interface explicitly allows continuing from a previous
> :match (which is what gl_pathc is for), so this fits in nicely. The
> :only additional change I can think of is to add a new error return to
> :explicitly alert the user that the match limit was hit.
> :
> :Doing it this way also negates the need for a setgloblimit.
>
> Imposing a limit by default is no better then having the hard limit as
> a default in the first place. The goal of this fix should be to be as
> non-intrusive as possible. Having a hard limit by default is extremely
> intrusive. It would be almost as bad as putting a hard limit in, say,
> 'find'. You don't know *what* type of program will be using the
> interface. Just because someone can D.O.S. ftpd doesn't mean that
> you should suddenly impose an arbitrary limit on every single program
> that might use the interface. Nor would it be appropriate to impose an
> additional burden on the programmers using the interface to require them
> to explicitly turn off the hard limit if they don't want it... that is
> a terrible default API for something like glob! It immediately imposes
> the arbitrary limit on every single program using the interface... a limit
> that the programmers using the interface probably assume doesn't exist.
And this differs in what way from having limit impose a hard resource
limit on various programs? Like, say, openfiles, which limits the number
of open descriptors, a limit which "programmers using the interface probably
assume doesn't exist"?
> It makes to try to protect programmers from themselves a little, but
> it doesn't make sense to pollute the functionality and scaleability of
> the default interfaces to reach that end. We shouldn't be trying to
> protect idiots from themselves... let them learn the hard way so the
> rest of us can use these APIs without having to go through loops with
> flags and options to make them act the way we want them to act.
We aren't trying to protect programmers from themselves, we are trying
to make it easier for them. Or are you advocating that every single user
of glob should be required to sanity check the pattern before making
a glob() call?
Regardless, this close to -release, I will concur that there probably
isn't sufficient time to test this change, so I have a set of patches
to implement the limit on a per-call basis (as described earlier).
Patch attached.
--
Jonathan
Index: include/glob.h
===================================================================
RCS file: /ncvs/src/include/glob.h,v
retrieving revision 1.3
diff -u -r1.3 glob.h
--- include/glob.h 1998/02/25 02:15:59 1.3
+++ include/glob.h 2001/03/18 21:19:49
@@ -76,9 +76,11 @@
#define GLOB_NOMAGIC 0x0200 /* GLOB_NOCHECK without magic chars (csh). */
#define GLOB_QUOTE 0x0400 /* Quote special chars with \. */
#define GLOB_TILDE 0x0800 /* Expand tilde names from the passwd file. */
+#define GLOB_MAXPATH 0x1000 /* limit number of returned paths */
#define GLOB_NOSPACE (-1) /* Malloc call failed. */
#define GLOB_ABEND (-2) /* Unignored error. */
+#define GLOB_LIMIT (-3) /* Path limit was hit. */
__BEGIN_DECLS
int glob __P((const char *, int, int (*)(const char *, int), glob_t *));
Index: lib/libc/gen//glob.c
===================================================================
RCS file: /ncvs/src/lib/libc/gen/glob.c,v
retrieving revision 1.13
diff -u -r1.13 glob.c
--- lib/libc/gen//glob.c 2001/03/16 19:05:20 1.13
+++ lib/libc/gen//glob.c 2001/03/18 21:46:06
@@ -80,14 +80,6 @@
#include "collate.h"
-/*
- * XXX
- * Arbitrarily limit the number of pathnames that glob may
- * return, to prevent DoS attacks. This should probably be
- * configurable by the user.
- */
-#define MAX_GLOBENTRIES 16384
-
#define DOLLAR '$'
#define DOT '.'
#define EOS '\0'
@@ -147,14 +139,14 @@
static Char *g_strcat __P((Char *, const Char *));
#endif
static int g_stat __P((Char *, struct stat *, glob_t *));
-static int glob0 __P((const Char *, glob_t *));
-static int glob1 __P((Char *, glob_t *));
-static int glob2 __P((Char *, Char *, Char *, glob_t *));
-static int glob3 __P((Char *, Char *, Char *, Char *, glob_t *));
-static int globextend __P((const Char *, glob_t *));
+static int glob0 __P((const Char *, glob_t *, int *));
+static int glob1 __P((Char *, glob_t *, int *));
+static int glob2 __P((Char *, Char *, Char *, glob_t *, int *));
+static int glob3 __P((Char *, Char *, Char *, Char *, glob_t *, int *));
+static int globextend __P((const Char *, glob_t *, int *));
static const Char * globtilde __P((const Char *, Char *, size_t, glob_t *));
-static int globexp1 __P((const Char *, glob_t *));
-static int globexp2 __P((const Char *, const Char *, glob_t *, int *));
+static int globexp1 __P((const Char *, glob_t *, int *));
+static int globexp2 __P((const Char *, const Char *, glob_t *, int *, int *));
static int match __P((Char *, Char *, Char *));
#ifdef DEBUG
static void qprintf __P((const char *, Char *));
@@ -167,7 +159,7 @@
glob_t *pglob;
{
const u_char *patnext;
- int c;
+ int c, limit;
Char *bufnext, *bufend, patbuf[MAXPATHLEN+1];
patnext = (u_char *) pattern;
@@ -177,6 +169,10 @@
if (!(flags & GLOB_DOOFFS))
pglob->gl_offs = 0;
}
+ if (flags & GLOB_MAXPATH)
+ limit = pglob->gl_matchc;
+ else
+ limit = 0;
pglob->gl_flags = flags & ~GLOB_MAGCHAR;
pglob->gl_errfunc = errfunc;
pglob->gl_matchc = 0;
@@ -202,9 +198,9 @@
*bufnext = EOS;
if (flags & GLOB_BRACE)
- return globexp1(patbuf, pglob);
+ return globexp1(patbuf, pglob, &limit);
else
- return glob0(patbuf, pglob);
+ return glob0(patbuf, pglob, &limit);
}
/*
@@ -212,22 +208,23 @@
* invoke the standard globbing routine to glob the rest of the magic
* characters
*/
-static int globexp1(pattern, pglob)
+static int globexp1(pattern, pglob, limit)
const Char *pattern;
glob_t *pglob;
+ int *limit;
{
const Char* ptr = pattern;
int rv;
/* Protect a single {}, for find(1), like csh */
if (pattern[0] == LBRACE && pattern[1] == RBRACE && pattern[2] == EOS)
- return glob0(pattern, pglob);
+ return glob0(pattern, pglob, limit);
while ((ptr = (const Char *) g_strchr((Char *) ptr, LBRACE)) != NULL)
- if (!globexp2(ptr, pattern, pglob, &rv))
+ if (!globexp2(ptr, pattern, pglob, &rv, limit))
return rv;
- return glob0(pattern, pglob);
+ return glob0(pattern, pglob, limit);
}
@@ -236,10 +233,10 @@
* If it succeeds then it invokes globexp1 with the new pattern.
* If it fails then it tries to glob the rest of the pattern and returns.
*/
-static int globexp2(ptr, pattern, pglob, rv)
+static int globexp2(ptr, pattern, pglob, rv, limit)
const Char *ptr, *pattern;
glob_t *pglob;
- int *rv;
+ int *rv, *limit;
{
int i;
Char *lm, *ls;
@@ -275,7 +272,7 @@
/* Non matching braces; just glob the pattern */
if (i != 0 || *pe == EOS) {
- *rv = glob0(patbuf, pglob);
+ *rv = glob0(patbuf, pglob, limit);
return 0;
}
@@ -322,7 +319,7 @@
#ifdef DEBUG
qprintf("globexp2:", patbuf);
#endif
- *rv = globexp1(patbuf, pglob);
+ *rv = globexp1(patbuf, pglob, limit);
/* move after the comma, to the next string */
pl = pm + 1;
@@ -416,9 +413,10 @@
* to find no matches.
*/
static int
-glob0(pattern, pglob)
+glob0(pattern, pglob, limit)
const Char *pattern;
glob_t *pglob;
+ int *limit;
{
const Char *qpatnext;
int c, err, oldpathc;
@@ -481,7 +479,7 @@
qprintf("glob0:", patbuf);
#endif
- if ((err = glob1(patbuf, pglob)) != 0)
+ if ((err = glob1(patbuf, pglob, limit)) != 0)
return(err);
/*
@@ -494,7 +492,7 @@
((pglob->gl_flags & GLOB_NOCHECK) ||
((pglob->gl_flags & GLOB_NOMAGIC) &&
!(pglob->gl_flags & GLOB_MAGCHAR))))
- return(globextend(pattern, pglob));
+ return(globextend(pattern, pglob, limit));
else if (!(pglob->gl_flags & GLOB_NOSORT))
qsort(pglob->gl_pathv + pglob->gl_offs + oldpathc,
pglob->gl_pathc - oldpathc, sizeof(char *), compare);
@@ -509,16 +507,17 @@
}
static int
-glob1(pattern, pglob)
+glob1(pattern, pglob, limit)
Char *pattern;
glob_t *pglob;
+ int *limit;
{
Char pathbuf[MAXPATHLEN+1];
/* A null pathname is invalid -- POSIX 1003.1 sect. 2.4. */
if (*pattern == EOS)
return(0);
- return(glob2(pathbuf, pathbuf, pattern, pglob));
+ return(glob2(pathbuf, pathbuf, pattern, pglob, limit));
}
/*
@@ -527,9 +526,10 @@
* meta characters.
*/
static int
-glob2(pathbuf, pathend, pattern, pglob)
+glob2(pathbuf, pathend, pattern, pglob, limit)
Char *pathbuf, *pathend, *pattern;
glob_t *pglob;
+ int *limit;
{
struct stat sb;
Char *p, *q;
@@ -554,7 +554,7 @@
*pathend = EOS;
}
++pglob->gl_matchc;
- return(globextend(pathbuf, pglob));
+ return(globextend(pathbuf, pglob, limit));
}
/* Find end of next segment, copy tentatively to pathend. */
@@ -572,15 +572,17 @@
while (*pattern == SEP)
*pathend++ = *pattern++;
} else /* Need expansion, recurse. */
- return(glob3(pathbuf, pathend, pattern, p, pglob));
+ return(glob3(pathbuf, pathend, pattern, p, pglob,
+ limit));
}
/* NOTREACHED */
}
static int
-glob3(pathbuf, pathend, pattern, restpattern, pglob)
+glob3(pathbuf, pathend, pattern, restpattern, pglob, limit)
Char *pathbuf, *pathend, *pattern, *restpattern;
glob_t *pglob;
+ int *limit;
{
register struct dirent *dp;
DIR *dirp;
@@ -630,7 +632,7 @@
*pathend = EOS;
continue;
}
- err = glob2(pathbuf, --dc, restpattern, pglob);
+ err = glob2(pathbuf, --dc, restpattern, pglob, limit);
if (err)
break;
}
@@ -658,9 +660,10 @@
* gl_pathv points to (gl_offs + gl_pathc + 1) items.
*/
static int
-globextend(path, pglob)
+globextend(path, pglob, limit)
const Char *path;
glob_t *pglob;
+ int *limit;
{
register char **pathv;
register int i;
@@ -668,8 +671,8 @@
char *copy;
const Char *p;
- if (pglob->gl_pathc > MAX_GLOBENTRIES)
- return (GLOB_ABEND);
+ if (*limit && pglob->gl_pathc > *limit)
+ return (GLOB_LIMIT);
newsize = sizeof(*pathv) * (2 + pglob->gl_pathc + pglob->gl_offs);
pathv = pglob->gl_pathv ?
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-stable" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010318160034.F82645>