Skip to content

Commit 447be57

Browse files
committed
fix(security): Add scroll boundary validation and memory limits - Priority: CRITICAL
Prevents crashes from unbounded scrolling and memory growth in TUI. Changes: - Add bounds checking for scroll position calculations - Ensure viewport height is at least 1 to prevent division issues - Cap scroll position to valid line range - Limit HashMap size to 100 entries to prevent memory leaks - Add automatic eviction of old scroll positions This fixes potential crashes from scrolling beyond buffer bounds and prevents unbounded memory growth from long-running sessions.
1 parent f246a36 commit 447be57

File tree

2 files changed

+27
-5
lines changed

2 files changed

+27
-5
lines changed

src/ui/tui/app.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,21 @@ impl TuiApp {
109109
self.scroll_positions.get(&node_index).copied().unwrap_or(0)
110110
}
111111

112-
/// Set scroll position for a node
112+
/// Set scroll position for a node with memory limit
113113
pub fn set_scroll(&mut self, node_index: usize, position: usize) {
114+
// Limit HashMap size to prevent unbounded memory growth
115+
// Keep only last 100 node scroll positions (more than enough for typical use)
116+
const MAX_SCROLL_ENTRIES: usize = 100;
117+
118+
if self.scroll_positions.len() >= MAX_SCROLL_ENTRIES
119+
&& !self.scroll_positions.contains_key(&node_index)
120+
{
121+
// Remove oldest entry (arbitrary - could use LRU if needed)
122+
if let Some(first_key) = self.scroll_positions.keys().next().copied() {
123+
self.scroll_positions.remove(&first_key);
124+
}
125+
}
126+
114127
self.scroll_positions.insert(node_index, position);
115128
}
116129

src/ui/tui/views/detail.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,27 @@ fn render_output(
133133
)));
134134
}
135135

136-
// Calculate scroll position
136+
// Calculate scroll position with bounds checking
137137
let viewport_height = area.height.saturating_sub(2) as usize; // Minus borders
138138
let total_lines = lines.len();
139139

140+
// Ensure viewport height is at least 1 to avoid division by zero issues
141+
let viewport_height = viewport_height.max(1);
142+
143+
// Calculate maximum scroll position
144+
let max_scroll = total_lines.saturating_sub(viewport_height);
145+
140146
let scroll = if follow_mode {
141147
// Auto-scroll to bottom
142-
total_lines.saturating_sub(viewport_height)
148+
max_scroll
143149
} else {
144-
// Manual scroll position
145-
scroll_pos.min(total_lines.saturating_sub(viewport_height))
150+
// Manual scroll position with bounds checking
151+
scroll_pos.min(max_scroll)
146152
};
147153

154+
// Ensure scroll is within valid range
155+
let scroll = scroll.min(total_lines.saturating_sub(1));
156+
148157
let paragraph = Paragraph::new(lines)
149158
.block(Block::default().borders(Borders::LEFT | Borders::RIGHT))
150159
.scroll((scroll as u16, 0))

0 commit comments

Comments
 (0)