[PATCH] ash: improve / fix glob expansion

[Felix Fietkau]

On 2017-01-31 21:29, 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.
>>
>> This can happen easily if a very long quoted string contains *, even
>> though no glob expansion should ever be performed on it (since it's
>> quoted).
>>
>> Fix this by properly parsing control characters and escaping and only
>> accept unquoted metacharacters. While we're at it, unify this check for
>> libc and built-in glob expansion
>>
>> Signed-off-by: Felix Fietkau <nbd at nbd.name>
>> ---
>>  shell/ash.c | 61 ++++++++++++++++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 44 insertions(+), 17 deletions(-)
>>
>> diff --git a/shell/ash.c b/shell/ash.c
>> index d8f41327b..642a2c727 100644
>> --- a/shell/ash.c
>> +++ b/shell/ash.c
>> @@ -7159,6 +7159,47 @@ addfname(const char *name)
>>         exparg.lastp = &sp->next;
>>  }
>>
>> +static int hasmeta(struct strlist *str)
>> +{
>> +       static const char chars[] ALIGN1 = {
>> +               '*', '?', '[', '\\', CTLQUOTEMARK, CTLESC, 0
>> +       };
>> +       static const char qchars[] ALIGN1 = {
>> +               CTLQUOTEMARK, CTLESC, 0
>> +       };
>> +       char *p;
>> +
>> +       /* Avoid glob() (and thus, stat() et al) for words like "echo" */
>> +       for (p = str->text; p && *p; p = strpbrk(p + 1, chars)) {
>> +               switch ((unsigned char) *p) {
>> +               case CTLQUOTEMARK:
>> +                       while (1) {
>> +                               p = strpbrk(p + 1, qchars);
>> +                               if (!p)
>> +                                       return 0;
>> +                               if (*p == CTLQUOTEMARK)
>> +                                       break;
>> +                               p++;
>> +                       }
>> +                       continue;
>> +               case '\\':
>> +               case CTLESC:
>> +                       if (!p[1])
>> +                               return 0;
>> +
>> +                       p++;
>> +                       continue;
>> +               case '*':
>> +               case '?':
>> +                       return 1;
>> +               case '[':
>> +                       return !!strchr(p + 1, ']');
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
> 
> I plan to massage it into this - do you see any bugs?
Looks okay.

> static int
> hasmeta(const char *p)
> {
>         static const char chars[] ALIGN1 = {
>                 '*', '?', '[', '\\', CTLQUOTEMARK, CTLESC, 0
>         };
> 
>         for (;;) {
>                 switch ((unsigned char) *p) {
>                 case '\0':
>                         return 0;
>                 case CTLQUOTEMARK:
>                         for (;;) {
>                                 p++;
>                                 if (*p == CTLQUOTEMARK)
>                                         break;
>                                 if (*p == CTLESC)
>                                         p++;
>                                 if (*p == '\0') /* huh? */
>                                         return 0;
>                         }
>                         break;
>                 case '\\':
>                 case CTLESC:
>                         p++;
>                         if (*p == '\0')
>                                 return 0;
>                         break;
>                 case '*':
>                 case '?':
>                         return 1;
>                 case '[':
>                         return strchr(p + 1, ']') != NULL;
>                 }
>                 p = strpbrk(p + 1, chars);
>                 if (!p)
>                         break;
>         }
> 
>         return 0;
> }
> 
> Oh. What about unclosed "[" followed by metachars? "file[weird_name.*"
I guess you could change it like this:

-			return strchr(p + 1, ']') != NULL;
+			if (strchr(p + 1, ']'))
+				return 1;
+			break;

- Felix