Skip to content

Commit

Permalink
Fix multiple vulnerabilities in the resolver
Browse files Browse the repository at this point in the history
This fixes some vulnerabilities in the resolver that make spoofing DNS
queries somewhat trivial due to the code failing to randomize xid, as
well as match the reply xid with the query, and the origin of the packet:

 - xid of the query was fixed at zero
 - xid from the reply was never checked
 - source address of the reply was never checked

This means anyone can flood the host with a fake reply with xid 0,
guessing the source port is trivial as it's less than 16bits (2^16 -
1024), which would cause odin to resolve a hostname to whatever an
attacker wanted.

While here also plug in two memory leaks.

Since this is CVE material, I've contacted @Kelimion before hand which
instructed to put it in a PR.

There are also more bugs as the code conflates answer section,
authority section and aditional section into one, while in reality
only the anwer section should be taken into consideration.
  • Loading branch information
haesbaert committed Feb 23, 2025
1 parent 940da61 commit 42d7e7a
Showing 1 changed file with 21 additions and 9 deletions.
30 changes: 21 additions & 9 deletions core/net/dns.odin
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,26 @@ package net
*/

/*
Copyright 2022 Tetralux <[email protected]>
Copyright 2022 Colin Davidson <[email protected]>
Copyright 2022 Jeroen van Rijn <[email protected]>.
Copyright 2024 Feoramund <[email protected]>.
Copyright 2022 Tetralux <[email protected]>
Copyright 2022 Colin Davidson <[email protected]>
Copyright 2022 Jeroen van Rijn <[email protected]>.
Copyright 2024 Feoramund <[email protected]>.
Copyright 2025 Christiano Haesbaert <[email protected]>.
Made available under Odin's BSD-3 license.
List of contributors:
Tetralux: Initial implementation
Colin Davidson: Linux platform code, OSX platform code, Odin-native DNS resolver
Jeroen van Rijn: Cross platform unification, code style, documentation
Feoramund: FreeBSD platform code
Haesbaert: Security fixes
*/

import "core:mem"
import "core:strings"
import "core:time"
import "core:os"
import "core:math/rand"
/*
Default configuration for DNS resolution.
*/
Expand Down Expand Up @@ -227,7 +230,7 @@ get_dns_records_from_nameservers :: proc(hostname: string, type: DNS_Record_Type
}

hdr := DNS_Header{
id = 0,
id = u16be(rand.uint32()),
is_response = false,
opcode = 0,
is_authoritative = false,
Expand Down Expand Up @@ -272,17 +275,23 @@ get_dns_records_from_nameservers :: proc(hostname: string, type: DNS_Record_Type
return nil, .Connection_Error
}

recv_sz, _ := recv_udp(conn, dns_response_buf[:]) or_continue
recv_sz, src := recv_udp(conn, dns_response_buf[:]) or_continue
if recv_sz == 0 {
continue
}
if src != name_server {
continue
}

dns_response = dns_response_buf[:recv_sz]

rsp, _ok := parse_response(dns_response, type)
rsp, xid, _ok := parse_response(dns_response, type)
if !_ok {
return nil, .Server_Error
}
if id != xid {
continue
}

if len(rsp) == 0 {
continue
Expand Down Expand Up @@ -803,7 +812,7 @@ parse_record :: proc(packet: []u8, cur_off: ^int, filter: DNS_Record_Type = nil)
- Data[]
*/

parse_response :: proc(response: []u8, filter: DNS_Record_Type = nil, allocator := context.allocator) -> (records: []DNS_Record, ok: bool) {
parse_response :: proc(response: []u8, filter: DNS_Record_Type = nil, allocator := context.allocator) -> (records: []DNS_Record, xid: u16be, ok: bool) {
context.allocator = allocator

HEADER_SIZE_BYTES :: 12
Expand All @@ -816,11 +825,13 @@ parse_response :: proc(response: []u8, filter: DNS_Record_Type = nil, allocator
dns_hdr_chunks := mem.slice_data_cast([]u16be, response[:HEADER_SIZE_BYTES])
hdr := unpack_dns_header(dns_hdr_chunks[0], dns_hdr_chunks[1])
if !hdr.is_response {
delete(_records)
return
}

question_count := int(dns_hdr_chunks[2])
if question_count != 1 {
delete(_records)
return
}
answer_count := int(dns_hdr_chunks[3])
Expand Down Expand Up @@ -872,6 +883,7 @@ parse_response :: proc(response: []u8, filter: DNS_Record_Type = nil, allocator
append(&_records, rec)
}
}
xid = hdr.id

return _records[:], true
return _records[:], xid, true
}

0 comments on commit 42d7e7a

Please sign in to comment.