Re: [Qemu-devel] [PATCH v2] qga: check length of command-line & environment variables

[P J P]

+-- On Wed, 22 May 2019, Marc-André Lureau wrote --+
| On Sun, May 19, 2019 at 10:55 AM P J P <address@hidden> wrote:
| > Qemu guest agent while executing user commands does not seem to
| > check length of argument list and/or environment variables passed.
| > It may lead to integer overflow or infinite loop issues. Add check
| > to avoid it.
| 
| Are you intentionally not telling where these overflow or loop happen?
| 
| Isn't the kernel already giving an error if given too much
| environment/arguments on exec?

Kernel would report error; But integer overflow would occur while computing 
'str_size' in a loop below, if count++ wraps around due to long list of 
arguments (or a loop) in 'strList *entry'. Negative 'count' would allocate 
large memory for 'args'

   args = g_malloc(count * sizeof(char *));

We don't have a reproducer. It does seem remote/unlikely, considering 
guest-agent is to be used by trusted parties to manage a guest.

| >      int count = 1, i = 0;  /* reserve for NULL terminator */
| > +    size_t str_size = 1, arg_max;
| >
| > +    arg_max = ga_get_arg_max();
| >      for (it = entry; it != NULL; it = it->next) {
| >          count++;
| >          str_size += 1 + strlen(it->value);
| > +        if (str_size >= arg_max || count >= arg_max / 2) {
| > +            break;
| 
| This seems to silently drop remaining arguments, which is probably not
| what you want.

Umnm, report an error and return?


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F