Mitigate Zip Bombs - #4535
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughHuman-readable size configuration and ZIP-bomb protection are added. ZIP extraction now validates archive limits, prevents traversal, streams real bytes, handles rejected files, and records failures. Configuration migrations, localized settings text, and unit/integration tests support the new behavior. ChangesZIP security and size configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review please. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tests/ImageProcessing/Photo/PhotoZipUploadTest.php (1)
144-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
\ZipArchive::OVERWRITEto prevent stale test state.Using
\ZipArchive::CREATEwithout\ZipArchive::OVERWRITEappends to the archive if the file already exists. If a previous test run aborts before cleanup, subsequent tests may fail due to the file containing leftover entries.
tests/ImageProcessing/Photo/PhotoZipUploadTest.php#L144-L147: combine the flags here:\ZipArchive::CREATE | \ZipArchive::OVERWRITE.tests/ImageProcessing/Photo/PhotoZipUploadTest.php#L160-L163: combine the flags here as well.tests/ImageProcessing/Photo/PhotoZipUploadTest.php#L175-L178: combine the flags here as well.tests/ImageProcessing/Photo/PhotoZipUploadTest.php#L202-L205: combine the flags here as well.tests/ImageProcessing/Photo/PhotoZipUploadTest.php#L223-L226: combine the flags here as well.♻️ Proposed fix
- if ($zip->open(TestConstants::SAMPLE_TEST_ZIP, \ZipArchive::CREATE) !== true) { + if ($zip->open(TestConstants::SAMPLE_TEST_ZIP, \ZipArchive::CREATE | \ZipArchive::OVERWRITE) !== true) {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e1ba5c8-8e52-4eed-b613-66e7cb7e5aa9
📒 Files selected for processing (42)
app/Actions/User/Create.phpapp/Actions/User/Save.phpapp/Assets/Helpers.phpapp/Enum/ConfigType.phpapp/Exceptions/Internal/ZipBombDetectedException.phpapp/Facades/Helpers.phpapp/Http/Resources/GalleryConfigs/UploadConfig.phpapp/Jobs/ExtractZip.phpapp/Models/Configs.phpapp/Repositories/ConfigManager.phpapp/Services/Zip/SafeZipExtractor.phpdatabase/migrations/2026_07_18_000000_add_zip_bomb_protection_configs.phpdatabase/migrations/2026_07_18_010000_convert_existing_configs_to_file_size_format.phpdatabase/migrations/2026_07_18_020000_add_zip_bomb_delete_rejected_file_config.phplang/ar/all_settings.phplang/bg/all_settings.phplang/cz/all_settings.phplang/de/all_settings.phplang/el/all_settings.phplang/en/all_settings.phplang/es/all_settings.phplang/fa/all_settings.phplang/fr/all_settings.phplang/hu/all_settings.phplang/it/all_settings.phplang/ja/all_settings.phplang/nl/all_settings.phplang/no/all_settings.phplang/pl/all_settings.phplang/pt/all_settings.phplang/ru/all_settings.phplang/sk/all_settings.phplang/sv/all_settings.phplang/tr/all_settings.phplang/vi/all_settings.phplang/zh_CN/all_settings.phplang/zh_TW/all_settings.phptests/ImageProcessing/Photo/PhotoZipUploadTest.phptests/Unit/HelpersUnitTest.phptests/Unit/Models/ConfigsTest.phptests/Unit/Repositories/ConfigManagerTest.phptests/Unit/Services/Zip/SafeZipExtractorTest.php
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
This does not warrant a CVE. At best it would be a 2.3
Summary by CodeRabbit