[PATCH] ash: improve / fix glob expansion

[Felix Fietkau]

On 2017-01-31 19:42, Denys Vlasenko wrote:
> On Sun, Jan 29, 2017 at 2:14 PM, Felix Fietkau <nbd at nbd.name> wrote:
>> When using musl libc glob() a very long string can cause glob() to fail,
>> which leads to an out of memory error being raised by ash.
> 
> But it works with glibc glob(), right?
It works, but it's rather inefficient.

> So other programs will also be affected.
I'm not aware of other programs passing arbitrary strings to glob() that
usually aren't filenames but sometimes might be.

> Let's fix it in musl (at least there;
I don't think musl needs to be fixed. You should never pass a string
longer than PATH_MAX to glob() anyway, since it's not possible to get a
valid result from that.

> your patch does make sense
> to use anyways since it prevents
> 
>     echo "*************"
> 
> from being needlessly passed to glob(), right?
Correct. I reproduced this bug with some shell code that was dealing
with long json strings that contain []. Passing those to glob()
definitely makes the code slower.

> commit 0dc99ac413d8bc054a2e95578475c7122455eee8
> Author: Rich Felker <dalias at aerifal.cx>
> Date:   Sun Jun 5 19:29:52 2011 -0400
> 
>     safety fix for glob's vla usage: disallow patterns longer than PATH_MAX
> 
>     this actually inadvertently disallows some valid patterns with
>     redundant / or * characters, but it's better than allowing unbounded
>     vla allocation.
> 
>     eventually i'll write code to move the pattern to the stack and
>     eliminate redundancy to ensure that it fits in PATH_MAX at the
>     beginning of glob. this would also allow it to be modified in place
>     for passing to fnmatch rather than copied at each level of recursion.
> 
> diff --git a/src/regex/glob.c b/src/regex/glob.c
> index 9a70f0b..67f84bc 100644
> --- a/src/regex/glob.c
> +++ b/src/regex/glob.c
> @@ -171,6 +171,8 @@ int glob(const char *pat, int flags, int
> (*errfunc)(const char *path, int err),
>                 d = "";
>         }
> 
> +       if (strlen(p) > PATH_MAX) return GLOB_NOSPACE;
> 
> 
> This sounds like musl having an implementation difficulty.
I'd say this is a reasonable restriction to simplify the implementation.

- Felix