| When we have a zero length for an embedded option, warn if any |
| more data or not the last option and return the number of |
| currently processed options. This fixes a potential buffer |
| overrun for options where the last embedded optional is an |
| optional string. Thanks to Paul Stewart @ Chromium for the fix. |
| |
| http://roy.marples.name/projects/dhcpcd/ci/732e88eaa0?sbs=0 |
| |
| --- dhcp-common.c |
| +++ dhcp-common.c |
| @@ -873,13 +873,24 @@ |
| /* Embedded options are always processed first as that |
| * is a fixed layout */ |
| n = 0; |
| for (i = 0, eopt = opt->embopts; i < opt->embopts_len; i++, eopt++) { |
| e = dhcp_optlen(eopt, ol); |
| - if (e == 0) |
| - /* Report error? */ |
| - return 0; |
| + if (e == 0) { |
| + /* An option was expected, but there is not enough |
| + * data for it. |
| + * This may not be an error as some options like |
| + * DHCP FQDN in RFC4702 have a string as the last |
| + * option which is optional. |
| + * FIXME: Add an flag to the options to indicate |
| + * wether this is allowable or not. */ |
| + if (ol != 0 || i + 1 < opt->embopts_len) |
| + logger(ctx, LOG_WARNING, |
| + "%s: %s: malformed option %d", |
| + ifname, __func__, opt->option); |
| + goto out; |
| + } |
| /* Use the option prefix if the embedded option |
| * name is different. |
| * This avoids new_fqdn_fqdn which would be silly. */ |
| if (!(eopt->type & RESERVED)) { |
| ov = strcmp(opt->var, eopt->var); |
| @@ -930,10 +941,11 @@ |
| od += eos + eol; |
| ol -= eos + eol; |
| } |
| } |
| |
| +out: |
| if (env) |
| free(pfx); |
| |
| /* Return number of options found */ |
| return n; |
| |
