dnsd: fix CRITICAL race in DNS transaction ID + DoS via compression loop
- Replace unsafe static mut DNS_TID (data race between loopback listener thread, mDNS responder thread, and scheme-call paths) with AtomicU16 + fetch_update. Eliminates torn writes and interleaved read-modify-write that could produce duplicate transaction IDs and misroute responses. - Add MAX_COMPRESSION_JUMPS=10 cap and backward-loop detection to decode_name. Previously a malicious DNS response with a self-referential or cyclic compression pointer could spin the scheme daemon's main thread indefinitely (DoS). Closes the two CRITICAL findings (C-19, C-20) from local/docs/NETWORKING-AND-DRIVERS-CODE-ASSESSMENT-2026-07-27.md §3.5
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
|
||||
use std::io;
|
||||
use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, UdpSocket};
|
||||
use std::sync::atomic::{AtomicU16, Ordering};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
// ─── DNS Wire Types (standalone, matching relibc's Dns/DnsQuery/DnsAnswer) ───
|
||||
@@ -39,22 +40,28 @@ pub(crate) struct DnsMessage {
|
||||
pub authorities: Vec<DnsAnswer>,
|
||||
}
|
||||
|
||||
/// Simple prng for transaction IDs (xorshift).
|
||||
static mut DNS_TID: u16 = 0x42;
|
||||
/// Thread-safe transaction ID counter (xorshift16) backed by `AtomicU16`.
|
||||
///
|
||||
/// `fetch_update` performs the read-modify-write as a single atomic step, which
|
||||
/// eliminates the data race that existed with the previous `static mut DNS_TID`
|
||||
/// shared between the loopback listener thread, the mDNS responder thread, and
|
||||
/// any scheme-call paths that originate queries concurrently.
|
||||
static DNS_TID: AtomicU16 = AtomicU16::new(0x42);
|
||||
|
||||
fn next_tid() -> u16 {
|
||||
// Simple xorshift16 for transaction IDs
|
||||
unsafe {
|
||||
let mut x = DNS_TID;
|
||||
x ^= x << 7;
|
||||
x ^= x >> 9;
|
||||
x ^= x << 8;
|
||||
if x == 0 {
|
||||
x = 0x42;
|
||||
}
|
||||
DNS_TID = x;
|
||||
x
|
||||
}
|
||||
DNS_TID
|
||||
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |prev| {
|
||||
let mut x = prev;
|
||||
// Simple xorshift16 PRNG — adequate for non-cryptographic transaction IDs.
|
||||
x ^= x << 7;
|
||||
x ^= x >> 9;
|
||||
x ^= x << 8;
|
||||
if x == 0 {
|
||||
x = 0x42;
|
||||
}
|
||||
Some(x)
|
||||
})
|
||||
.unwrap_or(0x42)
|
||||
}
|
||||
|
||||
impl DnsMessage {
|
||||
@@ -233,11 +240,14 @@ pub(crate) fn encode_name_to_vec(name: &str) -> Vec<u8> {
|
||||
data
|
||||
}
|
||||
|
||||
const MAX_COMPRESSION_JUMPS: u8 = 10;
|
||||
|
||||
fn decode_name(data: &[u8], pos: &mut usize) -> Result<String, String> {
|
||||
let name_ptr = 0b1100_0000u8;
|
||||
let mut name = String::new();
|
||||
let mut jumped = false;
|
||||
let mut jump_to = None;
|
||||
let mut jumps_remaining = MAX_COMPRESSION_JUMPS;
|
||||
|
||||
loop {
|
||||
if *pos >= data.len() {
|
||||
@@ -246,11 +256,16 @@ fn decode_name(data: &[u8], pos: &mut usize) -> Result<String, String> {
|
||||
|
||||
let len = data[*pos];
|
||||
if len & name_ptr == name_ptr {
|
||||
// Compression pointer
|
||||
if *pos + 2 > data.len() {
|
||||
return Err("truncated in compression".into());
|
||||
}
|
||||
if jumps_remaining == 0 {
|
||||
return Err("compression pointer loop".into());
|
||||
}
|
||||
let offset = u16::from_be_bytes([data[*pos] & !name_ptr, data[*pos + 1]]) as usize;
|
||||
if offset >= *pos && offset != *pos {
|
||||
return Err("compression pointer would loop backward".into());
|
||||
}
|
||||
if offset >= data.len() {
|
||||
return Err("invalid compression offset".into());
|
||||
}
|
||||
@@ -259,6 +274,7 @@ fn decode_name(data: &[u8], pos: &mut usize) -> Result<String, String> {
|
||||
}
|
||||
*pos = offset;
|
||||
jumped = true;
|
||||
jumps_remaining -= 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user