annotate files/patch-src_client_ns__turn__msg.c @ 13:c7cf16351c81

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
author Franz Glasner <fzglas.hg@dom66.de>
date Sat, 28 Mar 2020 15:44:52 +0100
parents
children
Ignore whitespace changes - Everywhere: Within whitespace: At end of lines:
rev   line source
13
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
1 --- src/client/ns_turn_msg.c.orig 2019-03-02 21:06:19 UTC
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
2 +++ src/client/ns_turn_msg.c
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
3 @@ -360,7 +360,13 @@ int stun_get_command_message_len_str(const u08bits* bu
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
4 {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
5 if (len < STUN_HEADER_LENGTH)
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
6 return -1;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
7 - return (int) (nswap16(((const u16bits*)(buf))[1]) + STUN_HEADER_LENGTH);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
8 + /* Validate the size the buffer claims to be */
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
9 + size_t bufLen = (size_t) (nswap16(((const u16bits*)(buf))[1]) + STUN_HEADER_LENGTH);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
10 + if (bufLen > len) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
11 + return -1;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
12 + }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
13 +
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
14 + return bufLen;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
15 }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
16
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
17 static int stun_set_command_message_len_str(u08bits* buf, int len) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
18 @@ -1351,10 +1357,34 @@ stun_attr_ref stun_attr_get_first_by_type_str(const u0
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
19 return NULL;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
20 }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
21
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
22 +static stun_attr_ref stun_attr_check_valid(stun_attr_ref attr, size_t remaining) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
23 +
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
24 + if(remaining >= 4) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
25 + /* Read the size of the attribute */
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
26 + size_t attrlen = stun_attr_get_len(attr);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
27 + remaining -= 4;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
28 +
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
29 + /* Round to boundary */
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
30 + uint16_t rem4 = ((uint16_t)attrlen) & 0x0003;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
31 + if(rem4) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
32 + attrlen = attrlen+4-rem4;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
33 + }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
34 +
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
35 + /* Check that there's enough space remaining */
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
36 + if(attrlen <= remaining) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
37 + return attr;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
38 + }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
39 + }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
40 +
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
41 + return NULL;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
42 +}
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
43 +
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
44 stun_attr_ref stun_attr_get_first_str(const u08bits* buf, size_t len) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
45
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
46 - if(stun_get_command_message_len_str(buf,len)>STUN_HEADER_LENGTH) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
47 - return (stun_attr_ref)(buf+STUN_HEADER_LENGTH);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
48 + int bufLen = stun_get_command_message_len_str(buf,len);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
49 + if(bufLen > STUN_HEADER_LENGTH) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
50 + stun_attr_ref attr = (stun_attr_ref)(buf+STUN_HEADER_LENGTH);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
51 + return stun_attr_check_valid(attr, bufLen - STUN_HEADER_LENGTH);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
52 }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
53
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
54 return NULL;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
55 @@ -1370,8 +1400,11 @@ stun_attr_ref stun_attr_get_next_str(const u08bits* bu
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
56 if(rem4) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
57 attrlen = attrlen+4-(int)rem4;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
58 }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
59 - const u08bits* attr_end=(const u08bits*)prev+4+attrlen;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
60 - if(attr_end<end) return attr_end;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
61 + /* Note the order here: operations on attrlen are untrusted as they may overflow */
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
62 + if(attrlen < end - (const u08bits*)prev - 4) {
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
63 + const u08bits* attr_end=(const u08bits*)prev+4+attrlen;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
64 + return stun_attr_check_valid(attr_end, end - attr_end);
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
65 + }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
66 return NULL;
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
67 }
c7cf16351c81 Apply patches for proper STUN message validation:
Franz Glasner <fzglas.hg@dom66.de>
parents:
diff changeset
68 }