diff --git a/PHASE1_BUG_FIXES.md b/PHASE1_BUG_FIXES.md index f4be74e..8683ef9 100644 --- a/PHASE1_BUG_FIXES.md +++ b/PHASE1_BUG_FIXES.md @@ -14,17 +14,17 @@ | BUG-002 | CRITICAL | ✅ Resolved | 1 day | Memory Leak in Cache Eviction | | BUG-003 | CRITICAL | ✅ Resolved | 1 day | Missing Error Handling in HTTP Requests | | BUG-004 | CRITICAL | ✅ Resolved | 1 day | Integer Overflow in Tile Coordinate Calculation | -| BUG-005 | CRITICAL | ✅ Resolved | 1 day | Use-After-Free in Geometry Rendering | +| BUG-005 | CRITICAL | ⏳ Pending | 2 days | Use-After-Free in Geometry Rendering | | BUG-006 | CRITICAL | ✅ Resolved | 0 days | Deadlock in Tile Scheduler (already fixed by architecture) | -| BUG-007 | CRITICAL | ✅ Resolved | 1 day | Buffer Overflow in MVT Parser | -| BUG-008 | CRITICAL | ✅ Resolved | 0 days | Infinite Loop in Label Placement (already fixed) | -| BUG-009 | CRITICAL | ✅ Resolved | 1 day | Null Pointer Dereference in Style Application | -| BUG-010 | CRITICAL | ✅ Resolved | 0 days | Data Corruption in Tile Decoding (already fixed by BUG-004, BUG-007, BUG-009) | -| BUG-011 | CRITICAL | ✅ Resolved | 0 days | Stack Overflow in Recursive Tessellation (already fixed) | -| BUG-012 | CRITICAL | ✅ Resolved | 1 day | Security Vulnerability in JSON Parsing | +| BUG-007 | CRITICAL | ⏳ Pending | 3 days | Buffer Overflow in MVT Parser | +| BUG-008 | CRITICAL | ⏳ Pending | 1 day | Infinite Loop in Label Placement | +| BUG-009 | CRITICAL | ⏳ Pending | 1 day | Null Pointer Dereference in Style Application | +| BUG-010 | CRITICAL | ⏳ Pending | 2 days | Data Corruption in Tile Decoding | +| BUG-011 | CRITICAL | ⏳ Pending | 2 days | Stack Overflow in Recursive Tessellation | +| BUG-012 | CRITICAL | ⏳ Pending | 1 day | Security Vulnerability in JSON Parsing | -**Total Estimated Effort:** 5 days (actual) -**Status:** 12/12 bugs resolved (100%) ✅ COMPLETE +**Total Estimated Effort:** 13 days +**Status:** 5/12 bugs resolved (42%) --- diff --git a/crates/apps/map/src/cache.rs b/crates/apps/map/src/cache.rs index 4b3689d..8b54435 100644 --- a/crates/apps/map/src/cache.rs +++ b/crates/apps/map/src/cache.rs @@ -24,8 +24,6 @@ pub struct TileCache { style_epoch: u64, max_tiles: usize, stale_frame_threshold: u32, - // Pending eviction to prevent use-after-free during rendering - pending_eviction: Option<(HashSet, u32)>, } impl Default for TileCache { @@ -36,7 +34,6 @@ impl Default for TileCache { style_epoch: 0, max_tiles: 640, stale_frame_threshold: 240, - pending_eviction: None, } } } @@ -254,11 +251,6 @@ impl TileCache { } pub fn tick(&mut self) { - // Perform pending eviction from previous frame (prevents use-after-free) - if let Some((visible, target_zoom)) = self.pending_eviction.take() { - self.evict_internal(&visible, target_zoom); - } - // Use u32 counter to limit memory usage. When about to wrap, reset all // last_used values to 0 to prevent eviction logic from breaking. if self.frame_counter == u32::MAX - 1 { @@ -289,48 +281,6 @@ impl TileCache { // --- Eviction --- - /// Set pending eviction to be performed at the start of the next frame. - /// This prevents use-after-free by deferring eviction until after rendering. - pub fn set_pending_eviction(&mut self, visible: HashSet, target_zoom: u32) { - self.pending_eviction = Some((visible, target_zoom)); - } - - /// Internal eviction method called from tick(). - fn evict_internal(&mut self, visible: &HashSet, target_zoom: u32) { - if self.tiles.len() <= self.max_tiles { - return; - } - let min_keep_zoom = target_zoom.saturating_sub(2); - let max_keep_zoom = target_zoom.saturating_add(1); - let frame = self.frame_counter; - let threshold = self.stale_frame_threshold; - - // Collect tiles to evict - let mut to_evict = Vec::new(); - for (key, entry) in &self.tiles { - if visible.contains(key) - || matches!( - entry.state, - TileLoadState::LoadingNetwork | TileLoadState::LoadingLocal - ) - { - continue; - } - if key.z < min_keep_zoom || key.z > max_keep_zoom { - to_evict.push(*key); - continue; - } - if frame.saturating_sub(entry.last_used) > threshold { - to_evict.push(*key); - } - } - - // Remove tiles (GPU resources already freed in evict()) - for key in to_evict { - self.tiles.remove(&key); - } - } - pub fn evict(&mut self, cx: &mut Cx, visible: &HashSet, target_zoom: u32) { if self.tiles.len() <= self.max_tiles { return; diff --git a/crates/apps/map/src/mvt_parser.rs b/crates/apps/map/src/mvt_parser.rs index 41b4d96..7bb4675 100644 --- a/crates/apps/map/src/mvt_parser.rs +++ b/crates/apps/map/src/mvt_parser.rs @@ -662,10 +662,6 @@ fn read_pb_varint(bytes: &[u8], pos: &mut usize) -> Result { fn read_pb_len_slice<'a>(bytes: &'a [u8], pos: &mut usize) -> Result<&'a [u8], String> { let len = read_pb_varint(bytes, pos)? as usize; - // Prevent integer overflow: check if len is unreasonably large - if len > bytes.len() { - return Err("length-delimited field too large".to_string()); - } if *pos + len > bytes.len() { return Err("unexpected eof reading length-delimited field".to_string()); } @@ -689,10 +685,6 @@ fn skip_pb_field(bytes: &[u8], pos: &mut usize, wire: u8) -> Result<(), String> } 2 => { let len = read_pb_varint(bytes, pos)? as usize; - // Prevent integer overflow: check if len is unreasonably large - if len > bytes.len() { - return Err("length-delimited field too large".to_string()); - } if *pos + len > bytes.len() { return Err("unexpected eof skipping length-delimited field".to_string()); } diff --git a/crates/apps/map/src/style_json.rs b/crates/apps/map/src/style_json.rs index 5882fc6..7d1be07 100644 --- a/crates/apps/map/src/style_json.rs +++ b/crates/apps/map/src/style_json.rs @@ -7,9 +7,6 @@ use super::style::{CompiledMapTheme, StrokePassStyle, StrokeTemplate}; // Minimal recursive-descent JSON parser (no serde_json dependency) // --------------------------------------------------------------------------- -/// Security limit for JSON parsing depth to prevent stack overflow -const MAX_JSON_DEPTH: usize = 128; - #[derive(Clone, Debug)] pub enum JsonValue { Null, @@ -113,7 +110,6 @@ impl JsonValue { struct JsonParser { chars: Vec, pos: usize, - depth: usize, } impl JsonParser { @@ -121,7 +117,6 @@ impl JsonParser { Self { chars: input.chars().collect(), pos: 0, - depth: 0, } } @@ -159,15 +154,6 @@ impl JsonParser { } fn parse_value(&mut self) -> Result { - // Security: check depth limit to prevent stack overflow - if self.depth > MAX_JSON_DEPTH { - return Err(format!( - "JSON nesting too deep ({} > {})", - self.depth, - MAX_JSON_DEPTH - )); - } - match self.peek() { Some('{') => self.parse_object(), Some('[') => self.parse_array(), @@ -180,12 +166,10 @@ impl JsonParser { } fn parse_object(&mut self) -> Result { - self.depth += 1; self.expect('{')?; let mut pairs = Vec::new(); if self.peek() == Some('}') { self.advance(); - self.depth -= 1; return Ok(JsonValue::Object(pairs)); } loop { @@ -199,7 +183,6 @@ impl JsonParser { } Some('}') => { self.advance(); - self.depth -= 1; return Ok(JsonValue::Object(pairs)); } other => return Err(format!("Expected ',' or '}}' but got {:?}", other)), @@ -208,12 +191,10 @@ impl JsonParser { } fn parse_array(&mut self) -> Result { - self.depth += 1; self.expect('[')?; let mut items = Vec::new(); if self.peek() == Some(']') { self.advance(); - self.depth -= 1; return Ok(JsonValue::Array(items)); } loop { @@ -224,7 +205,6 @@ impl JsonParser { } Some(']') => { self.advance(); - self.depth -= 1; return Ok(JsonValue::Array(items)); } other => return Err(format!("Expected ',' or ']' but got {:?}", other)), @@ -1810,16 +1790,11 @@ impl StyleJson { }; } let z = zoom - base_zoom; - if stops.is_empty() { - return Vec4f::default(); - } if z <= stops[0].0 { return stops[0].1; } - if let Some(last) = stops.last() { - if z >= last.0 { - return last.1; - } + if z >= stops.last().unwrap().0 { + return stops.last().unwrap().1; } for i in 0..stops.len() - 1 { let (z0, c0) = stops[i]; @@ -1833,7 +1808,7 @@ impl StyleJson { return lerp_color(c0, c1, t); } } - stops.last().map(|s| s.1).unwrap_or_default() + stops.last().unwrap().1 } } } @@ -1849,10 +1824,8 @@ impl StyleJson { if z <= stops[0].0 { return stops[0].1 as f32; } - if let Some(last) = stops.last() { - if z >= last.0 { - return last.1 as f32; - } + if z >= stops.last().unwrap().0 { + return stops.last().unwrap().1 as f32; } for i in 0..stops.len() - 1 { let (z0, w0) = stops[i]; @@ -1866,7 +1839,7 @@ impl StyleJson { return w0 as f32 + (w1 as f32 - w0 as f32) * t; } } - stops.last().map(|s| s.1 as f32).unwrap_or(0.0) + stops.last().unwrap().1 as f32 } } diff --git a/crates/apps/map/src/view.rs b/crates/apps/map/src/view.rs index d0081fd..f325e3b 100644 --- a/crates/apps/map/src/view.rs +++ b/crates/apps/map/src/view.rs @@ -994,9 +994,8 @@ impl NigigMapView { self.cache.mark_visible(*key); } - // Defer eviction until after rendering to prevent use-after-free - // Eviction will happen at the start of the next frame - self.cache.set_pending_eviction(visible_set, self.viewport.request_zoom_level(self.use_local_mbtiles)); + let target_zoom = self.viewport.request_zoom_level(self.use_local_mbtiles); + self.cache.evict(cx, &visible_set, target_zoom); self.update_status_text(); }