From: Martin Willi <martin@revosec.ch>
Date: Fri, 11 Jan 2013 13:45:32 +0000 (+0100)
Subject: Don't use bio_writer_t.skip() to write length field when appending more data
X-Git-Tag: 5.0.2dr4~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=54a1a75b2f5f3a9419eb5c18c07173827d5c9b39;p=thirdparty%2Fstrongswan.git

Don't use bio_writer_t.skip() to write length field when appending more data

If the writer reallocates its buffer, the length pointer might not be valid
anymore, or even worse, point to an arbitrary allocation.
---

diff --git a/src/libcharon/encoding/payloads/eap_payload.c b/src/libcharon/encoding/payloads/eap_payload.c
index dd2e257950..f2f35aa69c 100644
--- a/src/libcharon/encoding/payloads/eap_payload.c
+++ b/src/libcharon/encoding/payloads/eap_payload.c
@@ -410,14 +410,15 @@ eap_payload_t *eap_payload_create_nak(u_int8_t identifier, eap_type_t type,
 	eap_type_t reg_type;
 	u_int32_t reg_vendor;
 	bio_writer_t *writer;
-	chunk_t length, data;
+	chunk_t data;
 	bool added_any = FALSE, found_vendor = FALSE;
 	eap_payload_t *payload;
 
 	writer = bio_writer_create(12);
 	writer->write_uint8(writer, EAP_RESPONSE);
 	writer->write_uint8(writer, identifier);
-	length = writer->skip(writer, 2);
+	/* write zero length, we update it once we know the length */
+	writer->write_uint16(writer, 0);
 
 	write_type(writer, EAP_NAK, 0, expanded);
 
@@ -453,10 +454,9 @@ eap_payload_t *eap_payload_create_nak(u_int8_t identifier, eap_type_t type,
 
 	/* set length */
 	data = writer->get_buf(writer);
-	htoun16(length.ptr, data.len);
+	htoun16(data.ptr + offsetof(eap_packet_t, length), data.len);
 
 	payload = eap_payload_create_data(data);
 	writer->destroy(writer);
 	return payload;
 }
-
diff --git a/src/libstrongswan/bio/bio_writer.h b/src/libstrongswan/bio/bio_writer.h
index 57a5c3d384..2ac4f3556d 100644
--- a/src/libstrongswan/bio/bio_writer.h
+++ b/src/libstrongswan/bio/bio_writer.h
@@ -126,8 +126,11 @@ struct bio_writer_t {
 	void (*wrap32)(bio_writer_t *this);
 
 	/**
-	 * Skips len bytes in the buffer before the next data is written, returns
-	 * a chunk covering the skipped bytes.
+	 * Skips len bytes in the buffer, return chunk of skipped data.
+	 *
+	 * The returned chunk is not valid after calling any other writer function
+	 * (except get_buf()), because a buffer reallocation might move the
+	 * internal buffer to a different memory location!
 	 *
 	 * @param len		number of bytes to skip
 	 * @return			chunk pointing to skipped bytes in the internal buffer