Re: [v2] Re: [RFC] l2tp/ipv6: support for L2TPv2 over UDP over IPv6

[Benjamin LaHaise <bcrl@kvack.org>]

On Tue, Apr 10, 2012 at 02:49:16PM -0400, Benjamin LaHaise wrote:
...
> Also, I found a bug in OpenL2TPd (1.8 I think): certain types of AVPs are 
> not accepted if they are hidden, despite the fact that RFC 2661 permits 
> hiding these AVPs.  The AVPs in question are: Assigned Tunnel ID, Challege, 
> Challenge Response and Assigned Session ID.

Here's the fix for OpenL2TPd 1.8.  The use-after-realloc() in l2tp_avp_hide() 
doesn't always work if the memory gets moved and glibc scribbles on 
orig_buffer.  There's also an interesting typo in l2tp_avp_message_decode().

		-ben

-- 
"Thought is the essence of where you are now."


--- openl2tp-1.8/l2tp_avp.c.xxx	2008-05-08 15:05:26.000000000 -0400
+++ openl2tp-1.8/l2tp_avp.c	2012-04-10 18:07:05.866208480 -0400
@@ -535,16 +535,18 @@ static int l2tp_avp_hide(void **buffer, 
 	 * and we just need to shift the data up 2 bytes.
 	 */
 	new_buffer_len = orig_buffer_len + 2 + pad + 16;
-	new_buffer = realloc(orig_buffer, new_buffer_len + L2TP_AVP_HEADER_LEN);
+	new_buffer = malloc(new_buffer_len + L2TP_AVP_HEADER_LEN);
 	if (new_buffer == NULL) {
 		return -ENOMEM;
 	}
-	memmove(new_buffer + L2TP_AVP_HEADER_LEN + 2, orig_buffer + L2TP_AVP_HEADER_LEN, orig_buffer_len - L2TP_AVP_HEADER_LEN);
+	memcpy(new_buffer, orig_buffer, L2TP_AVP_HEADER_LEN);
+	memcpy(new_buffer + L2TP_AVP_HEADER_LEN + 2, orig_buffer + L2TP_AVP_HEADER_LEN, orig_buffer_len - L2TP_AVP_HEADER_LEN);
 	orig_len = new_buffer + L2TP_AVP_HEADER_LEN;
 	*orig_len = htons(orig_buffer_len - L2TP_AVP_HEADER_LEN);
 	if (new_buffer != orig_buffer) {
 		*buffer = new_buffer;
 	}
+	free(orig_buffer);
 	flag_len = new_buffer;
 	tmp = ntohs(*flag_len);
 	*flag_len = htons(tmp + 2 + pad);
@@ -1995,7 +1997,7 @@ int l2tp_avp_message_decode(int msg_len,
 				result = l2tp_avp_unhide(avp, &unhidden_avp_len, 
 							 (unsigned char *const) secret, secret_len,
 							 (unsigned char *const) data[TYPE(RANDOM_VECTOR)].value, 
-							 data[TYPE(RANDOM_VECTOR].value_len));
+							 data[TYPE(RANDOM_VECTOR)].value_len);
 				if (result < 0) {
 					l2tp_tunnel_log(tunnel, L2TP_AVPHIDE, LOG_ERR, "AVPHIDE: tunl %hu: avp unhide error: %s", 
 							l2tp_tunnel_id(tunnel), l2tp_strerror(-result));