Skip to content

Commit 7542f7b

Browse files
authored
Merge pull request #72 from adrgs/fix/security-audit-aisafe
fix: address aisafe.io security audit findings
2 parents fb39b70 + ccb6044 commit 7542f7b

12 files changed

Lines changed: 300 additions & 133 deletions

File tree

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Uses cargo-chef for optimized dependency caching
33

44
# Stage 1: Chef - install cargo-chef
5-
FROM rust:1.85-slim-bookworm AS chef
5+
FROM rust:1.88-slim-bookworm AS chef
66
RUN apt-get update && apt-get install -y --no-install-recommends \
77
libssl-dev \
88
pkg-config \

src/src/cache/mod.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -418,13 +418,13 @@ impl Cache {
418418
let mut empty_lists: Vec<String> = Vec::new();
419419

420420
if let Ok(mut store) = self.request_store.write() {
421+
// Collect keys ONCE outside the while loop to avoid O(N*M) re-cloning
422+
let keys: Vec<String> = store.keys().cloned().collect();
423+
421424
// Keep evicting until we've freed enough memory
422425
while freed < bytes_to_free {
423426
let mut made_progress = false;
424427

425-
// Get all keys (we need to collect to avoid borrow issues)
426-
let keys: Vec<String> = store.keys().cloned().collect();
427-
428428
// Round-robin: pop one oldest request from each list
429429
for key in &keys {
430430
if freed >= bytes_to_free {
@@ -490,10 +490,14 @@ impl Clone for Cache {
490490
/// Extract subdomain from cache key for size tracking
491491
fn extract_subdomain_from_key(key: &str) -> Option<String> {
492492
// files:{subdomain} -> subdomain
493-
// dns:{subdomain} -> subdomain
493+
// file:{subdomain}:{path} -> subdomain
494494
if key.starts_with("files:") || key.starts_with("file:") {
495495
return Some(key.split(':').nth(1)?.to_string());
496496
}
497+
// request:{subdomain}:{id} -> subdomain
498+
if key.starts_with("request:") {
499+
return Some(key.split(':').nth(1)?.to_string());
500+
}
497501
if key.starts_with("dns:") {
498502
// dns:{subdomain} or dns:{type}:{domain}
499503
let parts: Vec<&str> = key.split(':').collect();
@@ -502,13 +506,17 @@ fn extract_subdomain_from_key(key: &str) -> Option<String> {
502506
}
503507
// dns:{type}:{domain} - extract subdomain from domain
504508
// e.g., dns:A:test.abc123.example.com. -> abc123
509+
// Strip trailing dot and base domain, then take the last remaining label
505510
if parts.len() >= 3 {
506-
let domain = parts[2];
507-
// Try to extract subdomain from domain name
508-
let domain_parts: Vec<&str> = domain.split('.').collect();
509-
if domain_parts.len() >= 2 {
510-
// Second-to-last part before the base domain might be the subdomain
511-
return Some(domain_parts[1].to_string());
511+
let domain = parts[2].trim_end_matches('.');
512+
if let Some(stripped) = domain.strip_suffix(&CONFIG.server_domain) {
513+
let stripped = stripped.trim_end_matches('.');
514+
// The last label in the remaining string is the subdomain
515+
if let Some(sub) = stripped.rsplit('.').next() {
516+
if sub.len() == CONFIG.subdomain_length {
517+
return Some(sub.to_string());
518+
}
519+
}
512520
}
513521
}
514522
}

src/src/dns/mod.rs

Lines changed: 85 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
use anyhow::{anyhow, Result};
22
use base64::{engine::general_purpose::STANDARD as BASE64, Engine as _};
33
use rand::seq::SliceRandom;
4-
use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr};
4+
use std::collections::HashMap;
5+
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
56
use std::str::FromStr;
6-
use std::sync::Arc;
7+
use std::sync::{Arc, Mutex};
8+
use std::time::Instant;
79
use tokio::net::UdpSocket;
810
use tokio::sync::broadcast;
9-
use tracing::{error, info};
11+
use tracing::{error, info, warn};
1012
use trust_dns_proto::op::{Message, MessageType, OpCode, Query, ResponseCode};
1113
use trust_dns_proto::rr::rdata::{CNAME, MX, TXT};
1214
use trust_dns_proto::rr::{Name, RData, Record, RecordType};
@@ -18,6 +20,54 @@ use crate::models::{CacheMessage, DnsRequestLog};
1820
use crate::utils::config::CONFIG;
1921
use crate::utils::{generate_request_id, get_current_timestamp, get_subdomain_from_hostname};
2022

23+
/// Maximum UDP DNS response size (RFC 1035)
24+
const MAX_UDP_RESPONSE_SIZE: usize = 512;
25+
26+
/// Maximum DNS queries per second per IP
27+
const DNS_RATE_LIMIT_PER_SECOND: u32 = 100;
28+
29+
/// Rate limiter cleanup interval (number of queries between cleanups)
30+
const RATE_LIMITER_CLEANUP_INTERVAL: u64 = 1000;
31+
32+
struct DnsRateLimiter {
33+
limits: Mutex<HashMap<IpAddr, (Instant, u32)>>,
34+
max_per_second: u32,
35+
}
36+
37+
impl DnsRateLimiter {
38+
fn new(max_per_second: u32) -> Self {
39+
Self {
40+
limits: Mutex::new(HashMap::new()),
41+
max_per_second,
42+
}
43+
}
44+
45+
/// Returns true if the request should be allowed, false if rate-limited.
46+
fn check(&self, ip: IpAddr) -> bool {
47+
let mut limits = self.limits.lock().unwrap_or_else(|e| e.into_inner());
48+
let now = Instant::now();
49+
50+
let entry = limits.entry(ip).or_insert((now, 0));
51+
52+
// If more than 1 second has passed, reset the counter
53+
if now.duration_since(entry.0).as_secs() >= 1 {
54+
entry.0 = now;
55+
entry.1 = 1;
56+
true
57+
} else {
58+
entry.1 += 1;
59+
entry.1 <= self.max_per_second
60+
}
61+
}
62+
63+
/// Remove stale entries older than 10 seconds
64+
fn cleanup(&self) {
65+
let mut limits = self.limits.lock().unwrap_or_else(|e| e.into_inner());
66+
let now = Instant::now();
67+
limits.retain(|_, (instant, _)| now.duration_since(*instant).as_secs() < 10);
68+
}
69+
}
70+
2171
pub struct Server {
2272
cache: Arc<Cache>,
2373
tx: Arc<broadcast::Sender<CacheMessage>>,
@@ -33,10 +83,24 @@ impl Server {
3383

3484
let socket = Arc::new(UdpSocket::bind(format!("0.0.0.0:{}", CONFIG.dns_port)).await?);
3585
let mut buf = vec![0u8; 512];
86+
let rate_limiter = Arc::new(DnsRateLimiter::new(DNS_RATE_LIMIT_PER_SECOND));
87+
let mut query_count: u64 = 0;
3688

3789
loop {
3890
match socket.recv_from(&mut buf).await {
3991
Ok((len, addr)) => {
92+
// Rate limit check
93+
if !rate_limiter.check(addr.ip()) {
94+
warn!("DNS rate limit exceeded for {}", addr.ip());
95+
continue;
96+
}
97+
98+
// Periodic cleanup of stale rate limiter entries
99+
query_count += 1;
100+
if query_count.is_multiple_of(RATE_LIMITER_CLEANUP_INTERVAL) {
101+
rate_limiter.cleanup();
102+
}
103+
40104
let data = buf[..len].to_vec();
41105
let cache = self.cache.clone();
42106
let tx = self.tx.clone();
@@ -97,6 +161,22 @@ async fn handle_dns_request(
97161
let response_bytes = response
98162
.to_bytes()
99163
.map_err(|e| anyhow!("Failed to serialize DNS response: {}", e))?;
164+
165+
// If UDP response exceeds 512 bytes, set TC (truncation) bit and strip answers
166+
// This forces the client to retry via TCP (which is not spoofable)
167+
let response_bytes = if response_bytes.len() > MAX_UDP_RESPONSE_SIZE {
168+
let mut truncated = response.clone();
169+
truncated.set_truncated(true);
170+
// Remove all answers to fit within 512 bytes - keep only header + question
171+
let empty_answers: Vec<Record> = Vec::new();
172+
truncated.insert_answers(empty_answers);
173+
truncated
174+
.to_bytes()
175+
.map_err(|e| anyhow!("Failed to serialize truncated DNS response: {}", e))?
176+
} else {
177+
response_bytes
178+
};
179+
100180
socket.send_to(&response_bytes, addr).await?;
101181

102182
Ok(())
@@ -201,20 +281,9 @@ async fn log_dns_request(
201281

202282
let request_json = serde_json::to_string(&request_log)?;
203283

204-
// Push request to list and get the new length to calculate the correct index
284+
// Push request to list
205285
let list_key = format!("requests:{subdomain}");
206-
let index = cache
207-
.rpush(&list_key, &request_json)
208-
.await?
209-
.saturating_sub(1);
210-
211-
// Store the index for this request ID (used by delete endpoint)
212-
cache
213-
.set(
214-
&format!("request:{subdomain}:{request_id}"),
215-
&index.to_string(),
216-
)
217-
.await?;
286+
cache.rpush(&list_key, &request_json).await?;
218287

219288
let message = CacheMessage {
220289
cmd: "new_request".to_string(),

src/src/http/routes.rs

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -182,21 +182,9 @@ pub async fn catch_all(
182182

183183
let request_json = serde_json::to_string(&request_log).unwrap_or_default();
184184

185-
// Push request to list and get the new length to calculate the correct index
185+
// Push request to list
186186
let list_key = format!("requests:{subdomain}");
187-
let index = match state.cache.rpush(&list_key, &request_json).await {
188-
Ok(len) => len.saturating_sub(1), // Index is length - 1 (0-based)
189-
Err(_) => 0,
190-
};
191-
192-
// Store the index for this request ID (used by delete endpoint)
193-
let _ = state
194-
.cache
195-
.set(
196-
&format!("request:{subdomain}:{request_id}"),
197-
&index.to_string(),
198-
)
199-
.await;
187+
let _ = state.cache.rpush(&list_key, &request_json).await;
200188

201189
let message = crate::models::CacheMessage {
202190
cmd: "new_request".to_string(),
@@ -368,6 +356,11 @@ async fn serve_file(
368356
}
369357
}
370358

359+
// Security headers: prevent XSS via served content
360+
// Applied after user headers so they cannot be overridden
361+
response = response.header("X-Content-Type-Options", "nosniff");
362+
response = response.header("Content-Security-Policy", "sandbox allow-scripts");
363+
371364
return response
372365
.body(Body::from(content))
373366
.unwrap_or_else(|_| {

0 commit comments

Comments
 (0)