# HG changeset patch # User Franz Glasner # Date 1585406692 -3600 # Node ID c7cf16351c81c6f4f1bb9fd70e979b8761ffce3c # Parent 9b2679041b717b3a5868e2fc2976112b16ae3b9b Apply patches for proper STUN message validation: 1. 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. commit 9b8baa805582ae66d2a1ed68483609f90fcfb4d0 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. commit 14cb1c94e7be98869f45678ba195a26796a797c4 3. Changed type from int to size_t to avoid warning. warning: comparison between signed and unsigned integer expressions commit 4722697645cf033de8cf4f34e4214af750746365 See also: https://github.com/coturn/coturn/pull/472 diff -r 9b2679041b71 -r c7cf16351c81 files/patch-src_client_ns__turn__msg.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/files/patch-src_client_ns__turn__msg.c Sat Mar 28 15:44:52 2020 +0100 @@ -0,0 +1,68 @@ +--- src/client/ns_turn_msg.c.orig 2019-03-02 21:06:19 UTC ++++ src/client/ns_turn_msg.c +@@ -360,7 +360,13 @@ int stun_get_command_message_len_str(const u08bits* bu + { + if (len < STUN_HEADER_LENGTH) + return -1; +- return (int) (nswap16(((const u16bits*)(buf))[1]) + STUN_HEADER_LENGTH); ++ /* Validate the size the buffer claims to be */ ++ size_t bufLen = (size_t) (nswap16(((const u16bits*)(buf))[1]) + STUN_HEADER_LENGTH); ++ if (bufLen > len) { ++ return -1; ++ } ++ ++ return bufLen; + } + + static int stun_set_command_message_len_str(u08bits* buf, int len) { +@@ -1351,10 +1357,34 @@ stun_attr_ref stun_attr_get_first_by_type_str(const u0 + 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 */ ++ size_t attrlen = stun_attr_get_len(attr); ++ remaining -= 4; ++ ++ /* Round to boundary */ ++ uint16_t rem4 = ((uint16_t)attrlen) & 0x0003; ++ if(rem4) { ++ attrlen = attrlen+4-rem4; ++ } ++ ++ /* Check that there's enough space remaining */ ++ if(attrlen <= remaining) { ++ return attr; ++ } ++ } ++ ++ return NULL; ++} ++ + stun_attr_ref stun_attr_get_first_str(const u08bits* 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; +@@ -1370,8 +1400,11 @@ stun_attr_ref stun_attr_get_next_str(const u08bits* bu + if(rem4) { + attrlen = attrlen+4-(int)rem4; + } +- const u08bits* attr_end=(const u08bits*)prev+4+attrlen; +- if(attr_end