aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristiano Haesbaert <haesbaert@haesbaert.org>2025-02-08 23:54:56 +0100
committerChristiano Haesbaert <haesbaert@haesbaert.org>2025-02-09 00:00:11 +0100
commit605527f9db6275b296b43428e4ae8440fd782241 (patch)
treecde3fbb8101c68d46a06b84a31647cddcc1239ec
parent0683a3d67296c3b8dfdef8b730e0543e7eb4385b (diff)
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.
-rw-r--r--core/net/dns.odin23
1 files 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])