From 14cb1c94e7be98869f45678ba195a26796a797c4 Mon Sep 17 00:00:00 2001
From: Feral Interactive <noreply@feralinteractive.com>
Date: Fri, 1 Nov 2019 11:37:29 +0000
Subject: [PATCH 1/2] Validate the size of the buffer in
 stun_get_command_message_len_str(). Without this the caller could read off
 the end of the underlying buffer if it receives a maliciously crafted packet
 with an invalid header size.

---
 src/client/ns_turn_msg.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/client/ns_turn_msg.c b/src/client/ns_turn_msg.c
index 60483120..889cf44b 100644
--- a/src/client/ns_turn_msg.c
+++ b/src/client/ns_turn_msg.c
@@ -360,7 +360,14 @@ int stun_get_command_message_len_str(const uint8_t* buf, size_t len)
 {
 	if (len < STUN_HEADER_LENGTH)
 		return -1;
-	return (int) (nswap16(((const uint16_t*)(buf))[1]) + STUN_HEADER_LENGTH);
+
+	/* Validate the size the buffer claims to be */
+	int bufLen = (int) (nswap16(((const uint16_t*)(buf))[1]) + STUN_HEADER_LENGTH);
+	if (bufLen > len) {
+		return -1;
+	}
+
+	return bufLen;
 }
 
 static int stun_set_command_message_len_str(uint8_t* buf, int len) {

From 9b8baa805582ae66d2a1ed68483609f90fcfb4d0 Mon Sep 17 00:00:00 2001
From: Feral Interactive <noreply@feralinteractive.com>
Date: Fri, 1 Nov 2019 11:38:12 +0000
Subject: [PATCH 2/2] Validate the size of an attribute before returning it to
 the caller. Previously this was being done in stun_attr_get_next_str() to
 check that the previous attribute didn't exceed the size of the underlying
 buffer, however by that point any maliciously crafted attributes would have
 already had their chance to attack the caller.

---
 src/client/ns_turn_msg.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/client/ns_turn_msg.c b/src/client/ns_turn_msg.c
index 889cf44b..b77ebae2 100644
--- a/src/client/ns_turn_msg.c
+++ b/src/client/ns_turn_msg.c
@@ -1358,10 +1358,34 @@ stun_attr_ref stun_attr_get_first_by_type_str(const uint8_t* buf, size_t len, ui
   return NULL;
 }
 
+static stun_attr_ref stun_attr_check_valid(stun_attr_ref attr, size_t remaining) {
+
+  if(remaining >= 4) {
+    /* Read the size of the attribute */
+    int attrlen = stun_attr_get_len(attr);
+    remaining -= 4;
+
+    /* Round to boundary */
+    uint16_t rem4 = ((uint16_t)attrlen) & 0x0003;
+    if(rem4) {
+      attrlen = attrlen+4-(int)rem4;
+    }
+
+    /* Check that there's enough space remaining */
+    if(attrlen <= remaining) {
+      return attr;
+    }
+  }
+
+  return NULL;
+}
+
 stun_attr_ref stun_attr_get_first_str(const uint8_t* buf, size_t len) {
 
-  if(stun_get_command_message_len_str(buf,len)>STUN_HEADER_LENGTH) {
-    return (stun_attr_ref)(buf+STUN_HEADER_LENGTH);
+  int bufLen = stun_get_command_message_len_str(buf,len);
+  if(bufLen > STUN_HEADER_LENGTH) {
+    stun_attr_ref attr = (stun_attr_ref)(buf+STUN_HEADER_LENGTH);
+    return stun_attr_check_valid(attr, bufLen - STUN_HEADER_LENGTH);
   }
 
   return NULL;
@@ -1377,8 +1401,11 @@ stun_attr_ref stun_attr_get_next_str(const uint8_t* buf, size_t len, stun_attr_r
     if(rem4) {
       attrlen = attrlen+4-(int)rem4;
     }
-    const uint8_t* attr_end=(const uint8_t*)prev+4+attrlen;
-    if(attr_end<end) return attr_end;
+    /* Note the order here: operations on attrlen are untrusted as they may overflow */
+    if(attrlen < end - (const uint8_t*)prev - 4) {
+      const uint8_t* attr_end=(const uint8_t*)prev+4+attrlen;
+      return stun_attr_check_valid(attr_end, end - attr_end);
+    }
     return NULL;
   }
 }