> On 7 Jul 2020, at 7:57 am, Jesper Wallin <[email protected]> wrote:
> 
> Hi all,
> 
> I received a segmentation fault from dhclient(8) upon boot and decided
> to investigate...  My system is running with vm.malloc_conf=CFGJUR and
> figured one of those options was the cause of the crash.  I noticed that
> the buffer which holds my config options contained a lot of junk at the
> end and learned that 'J' is to blame together with a missing \0.
> 
> 
> How to reproduce:
> # sysctl vm.malloc_conf=J
> # cp /etc/dhclient.conf /etc/dhclient.conf.backup
> # echo 'supersede domain-name "ifconfig.se";' > /etc/dhclient.conf
> 
> Then run 'dhclient if0' a lot of times until it crashes, sometimes it
> takes more than 100 attempts.  Using vm.malloc_conf=CFGJUR might trigger
> it faster.
> 
> 
> In clparse.c:916, malloc(3) is used to get a buffer of the same length
> as the option in the config file.  But with 'J' in vm.malloc_conf, the
> buffer is bigger and contains junk.  I wouldn't say that my fix is the
> prettiest, but I get an extra byte and zero out the buffer.  Maybe
> someone has a more elegant fix for this.
> 
> 
> Yours,
> Jesper Wallin

you might want to put the memset after the check to see if the malloc failed...

> Index: clparse.c
> ===================================================================
> RCS file: /cvs/src/sbin/dhclient/clparse.c,v
> retrieving revision 1.199
> diff -u -p -r1.199 clparse.c
> --- clparse.c 13 May 2020 20:55:41 -0000      1.199
> +++ clparse.c 6 Jul 2020 21:25:54 -0000
> @@ -913,7 +913,8 @@ parse_option(FILE *cfile, int *code, str
>       } while (*fmt == 'A' && token == ',');
> 
>       free(options[i].data);
> -     options[i].data = malloc(hunkix);
> +     options[i].data = malloc(hunkix+1);
> +     memset(options[i].data, 0, hunkix+1);
>       if (options[i].data == NULL)
>               fatal("option data");
>       memcpy(options[i].data, hunkbuf, hunkix);
> 

Reply via email to