🛡️ Sentinel: [MEDIUM] Fix Unrestricted Audio File Loading (DoS)

.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 - Unrestricted Audio File Loading (DoS)
+**Vulnerability:** `AudioFeedback` loaded entire audio files into memory without size checks, allowing a potential Memory Exhaustion DoS via large configured file paths.
+**Learning:** Implicit trust in local configuration files can still lead to resource exhaustion if user input (file paths) points to massive resources.
+**Prevention:** Enforce strict resource limits (e.g., file size caps) on all loaded assets, even those specified in local configuration.

src/chirp/audio_feedback.py

@@ -25,6 +25,9 @@
     winsound = None  # type: ignore[assignment]
 
 
+MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024  # 5 MB
+
+
 class AudioFeedback:
     def __init__(
         self,
@@ -130,6 +133,13 @@ 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)
+        file_size = path.stat().st_size
+        if file_size > MAX_AUDIO_FILE_SIZE_BYTES:
+            raise ValueError(
+                f"Audio file '{path.name}' too large ({file_size} > {MAX_AUDIO_FILE_SIZE_BYTES} bytes)"
+            )
+
         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,9 @@ 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
+            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 +112,9 @@ 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
+            data = af._load_and_cache(Path("/fake/sound.wav"), key)
 
         audio_data, samplerate = data
 

tests/test_audio_security.py

@@ -0,0 +1,39 @@
+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.winsound", None)
+    def test_load_and_cache_rejects_large_files(self):
+        """_load_and_cache should raise ValueError if file exceeds size limit."""
+        af = AudioFeedback(logger=self.mock_logger, enabled=True)
+
+        # Mock Path.stat to return a large size
+        large_size = 6 * 1024 * 1024  # 6MB
+
+        # We patch Path.stat so that any Path object's stat() method returns our mock
+        with patch.object(Path, "stat") as mock_stat:
+            mock_stat.return_value.st_size = large_size
+
+            # We don't need to mock wave.open because it should fail BEFORE opening the file
+            # If it tries to open, it means the security check failed (or doesn't exist)
+            # and potentially would raise FileNotFoundError since the file doesn't exist,
+            # but we want to assert it raises ValueError specifically due to size.
+
+            # To distinguish from FileNotFoundError (if it proceeds to open),
+            # we rely on the specific ValueError expectation.
+
+            with self.assertRaises(ValueError) as cm:
+                af._load_and_cache(Path("fake_large_file.wav"), "key")
+
+            self.assertIn("too large", str(cm.exception))
+
+if __name__ == "__main__":
+    unittest.main()