Mercurial > hgrepos > FreeBSD > ports > net > turnserver
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 |
| 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 } |
