diff --git a/.jules/sentinel.md b/.jules/sentinel.md index de6782e..939de8b 100644 --- a/.jules/sentinel.md +++ b/.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. diff --git a/src/chirp/audio_feedback.py b/src/chirp/audio_feedback.py index 45ca0a4..9fbe17c 100644 --- a/src/chirp/audio_feedback.py +++ b/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: diff --git a/tests/test_audio_feedback.py b/tests/test_audio_feedback.py index c656333..94c92c4 100644 --- a/tests/test_audio_feedback.py +++ b/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 diff --git a/tests/test_audio_feedback_cache.py b/tests/test_audio_feedback_cache.py index 5bc3f3e..e8a5cc6 100644 --- a/tests/test_audio_feedback_cache.py +++ b/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 diff --git a/tests/test_audio_security.py b/tests/test_audio_security.py new file mode 100644 index 0000000..15822b8 --- /dev/null +++ b/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()