From 605527f9db6275b296b43428e4ae8440fd782241 Mon Sep 17 00:00:00 2001 From: Christiano Haesbaert Date: Sat, 8 Feb 2025 23:54:56 +0100 Subject: Fix some compression bugs in dns. - A compression pointer is when the two higher bits are set, the code was considering only 0xC0 as a pointer, where in reality anything from 0xC0-0xFF is a pointer, probably went unnoticed since you need big packets to have long pointers. - Make sure we can access the lower byte of the pointer by checking len, the code was careful to not access past the first byte, but ignored the second. - As per RFC9267 make sure a pointer only points backwards, this one is not so bad, as the code had a iteration_max that ended up guarding against infinite jumps. Lightly tested, some eyes are welcome, but these are remote DOSable. --- core/net/dns.odin | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/core/net/dns.odin b/core/net/dns.odin index 6d5dfea23..3730b8e94 100644 --- a/core/net/dns.odin +++ b/core/net/dns.odin @@ -533,18 +533,21 @@ decode_hostname :: proc(packet: []u8, start_idx: int, allocator := context.alloc return } - if packet[cur_idx] > 63 && packet[cur_idx] != 0xC0 { - return - } - - switch packet[cur_idx] { + switch { - // This is a offset to more data in the packet, jump to it - case 0xC0: + // A pointer is when the two higher bits are set. + case packet[cur_idx] & 0xC0 == 0xC0: + if len(packet[cur_idx:]) < 2 { + return + } pkt := packet[cur_idx:cur_idx+2] val := (^u16be)(raw_data(pkt))^ offset := int(val & 0x3FFF) - if offset > len(packet) { + // RFC 9267 a ptr should only point backwards, enough to avoid infinity. + // "The offset at which this octet is located must be smaller than the offset + // at which the compression pointer is located". Still keep iteration_max to + // avoid tiny jumps. + if offset > len(packet) || offset >= cur_idx { return } @@ -555,6 +558,10 @@ decode_hostname :: proc(packet: []u8, start_idx: int, allocator := context.alloc level += 1 } + // Validate label len + case packet[cur_idx] > LABEL_MAX: + return + // This is a label, insert it into the hostname case: label_size := int(packet[cur_idx]) -- cgit v1.2.3