> 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);
>