🛡️ Sentinel: [MEDIUM] Fix unbounded audio file read (DoS risk)

.jules/sentinel.md

@@ -7,3 +7,8 @@
 **Vulnerability:** Control characters could be injected via `word_overrides` configuration, bypassing initial input sanitization.
 **Learning:** Initial input sanitization is insufficient when configuration data (overrides) can re-introduce unsafe characters during processing.
 **Prevention:** Implement "Output Sanitization" as a final step in data processing pipelines. Ensure sanitization logic is reusable and safe (e.g., does not unintentionally destroy formatting like trailing whitespace unless intended).
+
+## 2025-05-21 - Unbounded Resource Loading (DoS)
+**Vulnerability:** `AudioFeedback` loaded user-defined sound files entirely into memory without size checks.
+**Learning:** Local apps are still vulnerable to DoS via configuration if they blindly trust file paths to fit in memory.
+**Prevention:** Enforce strict file size limits (e.g., `MAX_AUDIO_FILE_SIZE_BYTES`) before opening user-provided files.

src/chirp/audio_feedback.py

@@ -5,7 +5,7 @@
 import wave
 from contextlib import contextmanager
 from pathlib import Path
-from typing import Any, Dict, Iterator, Optional, Tuple, Union
+from typing import Any, Dict, Iterator, Optional
 
 from importlib import resources
 
@@ -25,6 +25,9 @@
     winsound = None  # type: ignore[assignment]
 
 
+MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024  # 5MB
+
+
 class AudioFeedback:
     def __init__(
         self,
@@ -130,6 +133,12 @@ def _play_sound(self, asset_name: str, override_path: Optional[str]) -> None:
             self._logger.exception("Failed to play sound %s: %s", asset_name, exc)
 
     def _load_and_cache(self, path: Path, key: str) -> Any:
+        # Security check: prevent loading massive files (DoS)
+        if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES:
+            raise ValueError(
+                f"Audio file {path} exceeds size limit of {MAX_AUDIO_FILE_SIZE_BYTES / 1024 / 1024:.1f}MB"
+            )
+
         if self._use_sounddevice:
             # Load as numpy array for volume-controlled playback via sounddevice
             with wave.open(str(path), "rb") as wf:

tests/test_audio_feedback.py

@@ -62,7 +62,10 @@ def test_load_and_cache_sounddevice(self, mock_wave, mock_np, mock_sd):
         mock_np.frombuffer.return_value = mock_audio_data
 
         key = "test_key"
-        data = af._load_and_cache(Path("/fake/sound.wav"), key)
+
+        with patch.object(Path, "stat") as mock_stat:
+            mock_stat.return_value.st_size = 1024  # Small size
+            data = af._load_and_cache(Path("/fake/sound.wav"), key)
 
         mock_wave.open.assert_called_with("/fake/sound.wav", "rb")
         mock_np.frombuffer.assert_called()
@@ -110,7 +113,10 @@ def test_load_and_cache_with_volume_scaling(self, mock_wave, mock_np, mock_sd):
         af = AudioFeedback(logger=self.mock_logger, enabled=True, volume=0.5)
 
         key = "test_key"
-        data = af._load_and_cache(Path("/fake/sound.wav"), key)
+
+        with patch.object(Path, "stat") as mock_stat:
+            mock_stat.return_value.st_size = 1024  # Small size
+            data = af._load_and_cache(Path("/fake/sound.wav"), key)
 
         audio_data, samplerate = data
 

tests/test_audio_feedback_cache.py

@@ -1,6 +1,5 @@
 import unittest
 from unittest.mock import MagicMock, patch
-from pathlib import Path
 import logging
 
 from chirp.audio_feedback import AudioFeedback

tests/test_audio_security.py

@@ -0,0 +1,44 @@
+import logging
+import unittest
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+from chirp.audio_feedback import AudioFeedback
+
+
+class TestAudioSecurity(unittest.TestCase):
+    def setUp(self):
+        self.mock_logger = MagicMock(spec=logging.Logger)
+
+    @patch("chirp.audio_feedback.sd", new=MagicMock())
+    @patch("chirp.audio_feedback.wave")
+    @patch("chirp.audio_feedback.np")
+    @patch("chirp.audio_feedback.winsound", None)
+    def test_rejects_large_files(self, mock_np, mock_wave):
+        """AudioFeedback should reject files larger than 5MB."""
+        af = AudioFeedback(logger=self.mock_logger, enabled=True)
+
+        # Mock Path.stat() to return a large size
+        with patch.object(Path, "stat") as mock_stat:
+            # 5MB + 1 byte
+            mock_stat.return_value.st_size = 5 * 1024 * 1024 + 1
+
+            # Mock wave.open to behave normally (if check fails, this would be called)
+            mock_wf = MagicMock()
+            mock_wave.open.return_value.__enter__.return_value = mock_wf
+            mock_wf.getframerate.return_value = 44100
+            mock_wf.getnchannels.return_value = 1
+            mock_wf.readframes.return_value = b"\x00"
+            mock_wf.getnframes.return_value = 1
+            mock_np.frombuffer.return_value = MagicMock()
+
+            # We expect ValueError. If the check is missing, this block will finish
+            # without raising ValueError (because we mocked wave.open), causing the test to fail.
+            with self.assertRaises(ValueError) as cm:
+                af._load_and_cache(Path("large_file.wav"), "key")
+
+            self.assertIn("exceeds size limit", str(cm.exception))
+
+
+if __name__ == "__main__":
+    unittest.main()