Repository navigation
Fix/security issues - #676
Closed
vipinkutthi-webkul wants to merge 6 commits into
Closed
vipinkutthi-webkul wants to merge 6 commits into
vipinkutthi-webkul wants to merge 6 commits into
Conversation
navneetkumar-pim-webkul
added a commit
that referenced
this pull request
Sep 10, 2026
… pick time Supersedes #676 on top of #690. `MediaContent::activeContentReason()` now covers .docx/.pptx (vbaProject.bin part), legacy .doc/.ppt (VBA stream) and RTF (\objupdate, \objautlink) beside the existing PDF markers, using the same head/tail windowed read so a large upload never lands in memory whole. `FileOrImageValidValue` reports the reason in the validation message. A new `admin.media.scan` endpoint runs the same rule on a file the moment the media widget picks it; the three widgets share one `$scanMedia` helper in app.js and only treat a 422 as a rejection, since form validation stays the enforcement point. Claude-Session: https://claude.ai/code/session_01JvM9wYbv339YvEnEuvsKrT
navneetkumar-pim-webkul
added a commit
that referenced
this pull request
Sep 10, 2026
* fix(core): scan Office and RTF uploads for macros, pre-check media at pick time Supersedes #676 on top of #690. `MediaContent::activeContentReason()` now covers .docx/.pptx (vbaProject.bin part), legacy .doc/.ppt (VBA stream) and RTF (\objupdate, \objautlink) beside the existing PDF markers, using the same head/tail windowed read so a large upload never lands in memory whole. `FileOrImageValidValue` reports the reason in the validation message. A new `admin.media.scan` endpoint runs the same rule on a file the moment the media widget picks it; the three widgets share one `$scanMedia` helper in app.js and only treat a 422 as a rejection, since form validation stays the enforcement point. Claude-Session: https://claude.ai/code/session_01JvM9wYbv339YvEnEuvsKrT * fix(core): scan Office and RTF uploads for macros, pre-check media at pick time Supersedes #676 on top of #690. `MediaContent::activeContentReason()` now covers .docx/.pptx (vbaProject.bin part), legacy .doc/.ppt (VBA stream) and RTF (\objupdate, \objautlink) beside the existing PDF markers, using the same head/tail windowed read so a large upload never lands in memory whole. `FileOrImageValidValue` reports the reason in the validation message. A new `admin.media.scan` endpoint runs the same rule on a file the moment the media widget picks it; the three widgets share one `$scanMedia` helper in app.js and only treat a 422 as a rejection, since form validation stays the enforcement point. Claude-Session: https://claude.ai/code/session_01JvM9wYbv339YvEnEuvsKrT * fix(admin): normalize media scan extensions (#694)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
store,update,testConnection): theextras.urlfield could overrideapi_urlat request time but was never checked againstSafeWebhookUrl, letting an authenticated admin route outbound webhook calls to internal hosts or cloud metadata endpoints.extras.urlis now validated the same way asapi_urlbefore create/update/test.ActiveContentScanner(packages/Webkul/Core/src/Rules/ActiveContentScanner.php) that detects embedded active content — JS/auto-actions in PDFs,<script>/inline event handlers/javascript:URIs in SVGs, and macros/auto-run objects in OOXML, legacy OLE, and RTF documents — and wired it intoFileOrImageValidValueso it runs on save for every file/image attribute upload.media/scanendpoint (MediaController::scan+MediaScanRequest) that runs the sameFileOrImageValidValuecheck at pick/drop time in the media widget (files, gallery, image components), giving admins immediate feedback before the file reaches save — this is a UX pre-check only, the save-time scan is the actual enforcement point.How To Test This?
extras.urlpointing at an internal/link-local address (e.g.http://169.254.169.254/) — the request should be rejected with the unsafe-api-url validation message. Repeat via "Test Connection". Runvendor/bin/pest packages/Webkul/Admin/tests/Feature/MagicAI/MagicAIPlatformSecurityTest.php./OpenActionor/JavaScriptentry, and an SVG containing<script>or anonloadattribute — both should be rejected with the "active content detected" message, both when picked in the media widget (via the newmedia/scanAJAX call) and again if the check is bypassed and save is attempted directly. Runvendor/bin/pest packages/Webkul/Admin/tests/Feature/MediaScanTest.phpandvendor/bin/pest packages/Webkul/Core/tests/Unit/FileOrImageValidValueSecurityTest.php.