From: krw <krw@openbsd.org>
Date: Sat, 21 Jul 2018 15:24:55 +0000 (+0000)
Subject: Reading past the end of a buffer is bad, Even if the
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=373f0df1f8c4f067f5a8ec780cd0c302831f1764;p=openbsd

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@
---

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