From c62b5d1894204b09b62d534d8ec649b58b6c7d4f Mon Sep 17 00:00:00 2001
From: "Alan T. DeKok" <aland@freeradius.org>
Date: Sat, 6 Apr 2019 08:54:08 -0400
Subject: [PATCH] fix TLV encoding

by adding the current TLV header, which was missing in the
previous commit.

Also clean up the code and use extend_option() where the data
is more than 255 bytes.

We try to avoid splitting data across multiple options.  The result
is that the encoder may output multiple options of the same number,
BUT where the lengths are smaller than 255.  This means that any
decoder can just decode the data in-place most of the time.

We split data across multiple options ONLY when the encoded data
is more than 255 bytes.  And even then, we try to start that
encoding on a new option.
---
 src/protocols/dhcpv4/encode.c | 59 ++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/src/protocols/dhcpv4/encode.c b/src/protocols/dhcpv4/encode.c
index 988490753121..c3baa941a3e7 100644
--- a/src/protocols/dhcpv4/encode.c
+++ b/src/protocols/dhcpv4/encode.c
@@ -303,8 +303,7 @@ static ssize_t encode_rfc_hdr(uint8_t *out, ssize_t outlen,
 			out = extend_option(out, end, p, len);
 			if (!out) break;
 
-			p = out + 2;
-			p += out[1];
+			p = out + 2 + out[1];
 		}
 
 		next = fr_cursor_current(cursor);
@@ -367,6 +366,7 @@ static ssize_t encode_tlv_hdr(uint8_t *out, ssize_t outlen,
 		if (len < 0) return len;
 		if (len == 0) break;		/* Insufficient space */
 
+
 		/*
 		 *	If the newly added data fits within the
 		 *	current option, then update the header, and go
@@ -377,41 +377,44 @@ static ssize_t encode_tlv_hdr(uint8_t *out, ssize_t outlen,
 			p += len;
 
 		} else {
-			p += len;
+			/*
+			 *	The data doesn't fit within the
+			 *	current option.  Start a new option.
+			 */
 
 			/*
-			 *	The encoder added more data than fits
-			 *	into the current option.  Wind the
-			 *	pointers to the end of the encoded
-			 *	data.
+			 *	Not enough space for the 2 byte
+			 *	header.
 			 */
-			while ((out + out[1]) < p) {
-				out += out[1]; /* should be 255 */
-			}
+			if ((p + 2 + len) >= end) break;
 
 			/*
-			 *	The current option ends at p, which is
-			 *	what we want.  However, if the current
-			 *	option ALSO is full, then we need to
-			 *	add a new option header.
+			 *	Move the data up and start a new
+			 *	option.
 			 */
-			if (out[1] == 255) {
-				out += out[1];
+			memmove(p + 2, p, len);
+			p[0] = start[0];
+			p[1] = 0;
+			out = p;
+			p = out + 2;
 
+			/*
+			 *	The data fits entirely within the new
+			 *	option.  Just use that.
+			 */
+			if (len <= 255) {
+				out[1] = len;
+				p += len;
+
+			} else {
 				/*
-				 *	Don't add in an option header.
-				 *	Rely on the caller to check
-				 *	that there's no more room in
-				 *	the buffer.
+				 *	The data has to be split
+				 *	across multiple options.
 				 */
-				if ((end - out) < 3) {
-					p = out;
-					break;
-				}
-
-				out[0] = start[0];
-				out[1] = 0;
-				p = out + 2;
+				out = extend_option(out, end, p, len);
+				if (!out) break;
+
+				p = out + 2 + out[1];
 			}
 		}