Re: expand: Fix buffer overflow in expandmeta

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Mon, Mar 26, 2018 at 06:37:45AM +0200, Harald van Dijk wrote:
>
> >Besides, even if we did impose a limit here
> >we'd still have to check that limit at every recursion level which
> >means that the code won't be that much simpler.
> 
> Proof of concept attached. The resulting code is about 100 bytes smaller at
> -Os than the dynamic reallocation approach. It's possible to further reduce
> that with a statically allocated buffer, but of course that means a constant
> memory cost at run time.

FWIW your patch when built with -O2 with gcc 4.7.2 is actually
16 bytes bigger than my version.

> @@ -1344,6 +1335,8 @@ expmeta(char *enddir, char *name)
>  			metaflag++;
>  		p = name;
>  		do {
> +			if (enddir == expdir + PATH_MAX)
> +				return;
>  			if (*p == '\\')
>  				p++;
>  			*enddir++ = *p;

Also there is another loop in between these two hunks that also
needs to check enddir.  My version doesn't need it because the
caller will ensure that at least FILENAME bytes are available.

> @@ -1390,22 +1383,34 @@ expmeta(char *enddir, char *name)
>  		if (dp->d_name[0] == '.' && ! matchdot)
>  			continue;
>  		if (pmatch(start, dp->d_name)) {
> +			p = enddir;
> +			cp = dp->d_name;
> +			for (;;) {
> +				if (p == expdir + PATH_MAX)
> +					goto toolong;
> +				if ((*p++ = *cp++) == '\0')
> +					break;
> +			}
>  			if (atend) {
> -				scopy(dp->d_name, enddir);
>  				addfname(expdir);
>  			} else {
> -				for (p = enddir, cp = dp->d_name;
> -				     (*p++ = *cp++) != '\0';)
> -					continue;
>  				p[-1] = '/';
> -				expmeta(p, endname);
> +				expmeta1(expdir, p, endname);
>  			}
>  		}
> +toolong: ;
>  	}
>  	closedir(dirp);
>  	if (! atend)
>  		endname[-esc - 1] = esc ? '\\' : '/';
>  }

Thanks,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html