From 01f4f5d95824749014ec790883cbed8aec443e83 Mon Sep 17 00:00:00 2001 From: vasilito Date: Mon, 27 Jul 2026 14:32:18 +0900 Subject: [PATCH] dnsd: fix CRITICAL race in DNS transaction ID + DoS via compression loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../redbear-dnsd/source/src/transport.rs | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/local/recipes/system/redbear-dnsd/source/src/transport.rs b/local/recipes/system/redbear-dnsd/source/src/transport.rs index 0e74ec4f5f..4b259eb4f9 100644 --- a/local/recipes/system/redbear-dnsd/source/src/transport.rs +++ b/local/recipes/system/redbear-dnsd/source/src/transport.rs @@ -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, } -/// 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 { data } +const MAX_COMPRESSION_JUMPS: u8 = 10; + fn decode_name(data: &[u8], pos: &mut usize) -> Result { 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 { 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 { } *pos = offset; jumped = true; + jumps_remaining -= 1; continue; }