On Sat, Jan 11, 2025 at 12:53 AM Stefano Brivio
<sbrivio(a)redhat.com> wrote:
On Fri, 10 Jan 2025 11:26:26 +0100
Enrique Llorente <ellorent(a)redhat.com> wrote:
@@ -162,17 +180,20 @@ static int fill(struct msg
*m)
* Put it there explicitly, unless requested via option 55.
*/
if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
- fill_one(m, 53, &offset);
+ offset = fill_one(m, 53, offset);
Now suppose that somebody adds a "long" option before this block, or...
for (i = 0; i < opts[55].clen; i++) {
o = opts[55].c[i];
if (opts[o].slen != -1)
- fill_one(m, o, &offset);
+ offset = fill_one(m, o, offset);
}
before this one, and fill_one() returns -1. Then...
for (o = 0; o < 255; o++) {
- if (opts[o].slen != -1 && !opts[o].sent)
- fill_one(m, o, &offset);
+ if (opts[o].slen != -1 && !opts[o].sent) {
+ offset = fill_one(m, o, offset);
you use offset as -1 here, and boom. If not, see directly below).
+ if (offset == -1)
Or maybe the domain name is actually too long, and offset is -1 here...
+ debug("DHCP:
skipping option %i", o);
+ }
}
m->o[offset++] = 255;
...and boom. This sets the last byte of "magic" (depending on the
architecture) to 0xff, making the whole message invalid.
well, I was expecting the only problematic options being at > 55
(didn't check the standard)
but I agree that we have to be consistent on handling the fill_in result.