From 373f0df1f8c4f067f5a8ec780cd0c302831f1764 Mon Sep 17 00:00:00 2001 From: krw Date: Sat, 21 Jul 2018 15:24:55 +0000 Subject: [PATCH] Reading past the end of a buffer is bad, Even if the extra byte is always there. Even if the byte contains innocuous data that isn't used. Eeven if a particular level of optimization of a particular compiler avoids it by processing things backwards. Bad. So simplify and correct logic. Perhaps even proof the code against future generations of clever compilers. Pointed out by Brandon Falk. Thanks! ok millert@ tb@ --- sbin/dhclient/options.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sbin/dhclient/options.c b/sbin/dhclient/options.c index 0782ed22f1a..5753fd01046 100644 --- a/sbin/dhclient/options.c +++ b/sbin/dhclient/options.c @@ -1,4 +1,4 @@ -/* $OpenBSD: options.c,v 1.109 2017/10/11 00:09:32 krw Exp $ */ +/* $OpenBSD: options.c,v 1.110 2018/07/21 15:24:55 krw Exp $ */ /* DHCP options parsing and reassembly. */ @@ -404,13 +404,19 @@ int parse_option_buffer(struct option_data *options, unsigned char *buffer, int length) { - unsigned char *s, *t, *end = buffer + length; + unsigned char *s, *t, *end; char *name, *fmt; int len, code; - for (s = buffer; *s != DHO_END && s < end; ) { + s = buffer; + end = s + length; + while (s < end) { code = s[0]; + /* End options terminate processing. */ + if (code == DHO_END) + break; + /* Pad options don't have a length - just skip them. */ if (code == DHO_PAD) { s++; -- 2.20.1