Skip to content

fix(media): Allow deleting files outside upload_files - #17029

Open
Ibochkarev wants to merge 1 commit into
modxcms:3.xfrom
Ibochkarev:fix/15905-delete-outside-upload-files
Open

Ibochkarev wants to merge 1 commit into
modxcms:3.xfrom
Ibochkarev:fix/15905-delete-outside-upload-files

Conversation

@Ibochkarev

Copy link
Copy Markdown
Collaborator

What changed and why

removeObject() called checkFileType() before delete(). When the media source property allowedFileTypes is empty, that method uses the system setting upload_files. The default list has no log, so deleting core/cache/logs/error.log failed with file_err_ext_not_allowed.

#15643 (SEC-3837) added the check so the manager cannot write a disallowed extension. The setting text in core/lexicon/en/setting.inc.php describes uploads. Delete does not write a new extension. Browser/File/Remove already requires the file_remove permission and the source remove policy.

The call is gone from removeObject(). createObject(), updateObject(), uploadObjectsToContainer(), and the new name in renameObject() still use checkFileType().

How to test

With allowedFileTypes empty on the Filesystem source, delete core/cache/logs/error.log from the file browser. It should be removed.

Upload or create a file whose extension is not in upload_files. That should still fail with file_err_ext_not_allowed. Renaming a file to a disallowed extension should fail the same way.

Related issue(s)/PR(s)

Resolves #15905

The check comes from #15643.

Compatibility notes

This applies to every media source that uses modMediaSource::removeObject(). There is no schema change and no upgrade script.

Breaking change assessment

No public API signature change. A user who already has file_remove can now delete a file whose extension is outside upload_files. Writing that extension is still rejected.

Test coverage

No test added. _build/test has no media source coverage for removeObject(), and there is no local properties.inc.php to run PHPUnit against a filesystem source.

Contributors

@Jako reported that error.log cannot be deleted and pointed at #15643.

AI tool use

Cursor compared removeObject() with the checks added in #15643 and removed the extension check from delete only.

removeObject() rejected any extension missing from upload_files, so
core/cache/logs/error.log could not be deleted. Delete does not write
a new extension. Create, upload, update, and rename still check the list.

Fixes modxcms#15905
@Ibochkarev
Ibochkarev marked this pull request as ready for review September 21, 2026 13:11
@Ibochkarev Ibochkarev added area-security bug The issue in the code or project, which should be addressed. labels Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-security bug The issue in the code or project, which should be addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't delete files with an extension not in upload_files

1 participant