Browse Source

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.
Christiano Haesbaert 7 months ago
parent
commit
605527f9db
1 changed files with 15 additions and 8 deletions
  1. 15 8
      core/net/dns.odin

+ 15 - 8
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])