- Issue created by @prudloff
- π«π·France prudloff Lille
I added and updated the patch from the private issue.
People from the "Fixed by" section of https://www.drupal.org/sa-core-2024-002 β should probably be credited. - πΊπΈUnited States smustgrave
Reading https://www.drupal.org/sa-core-2024-002 β believe this is a good coverage.
- πΊπΈUnited States xjm
I don't think this paragraph contains sufficient information to determine if it is good test coverage or not:
Under certain uncommon site configurations, a bug in the CKEditor 5 module can cause some image uploads to move the entire webroot to a different location on the file system. This could be exploited by a malicious user to take down a site.
The commit corresponding to the SA might be more helpful to review.
Ideally, it would be reviewed by someone with access to the original private issue report (which I've added to the summary). Reviewing it might be a good task for one of our provisional Security Team members.
- πΊπΈUnited States xjm
Another step I would expect to see for reviewing this would to be to revert the commit that fixed SA-CORE-2025-002, and have the test fail as expected, and then document that failure here on the issue.
- πΊπΈUnited States benjifisher Boston area
xjm β credited benjifisher β .
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
xjm β credited kim.pepper β .
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
xjm β credited larowlan β .
- πΊπΈUnited States smustgrave
Uploaded an MR with the fix reverted and the unit test added fails
Fail 0.117s testInvalidFile Log 0.442s *** Process execution output *** PHPUnit 10.5.46 by Sebastian Bergmann and contributors. Runtime: PHP 8.3.22 Configuration: /builds/issue/drupal-3527408/core/phpunit.xml.dist F 1 / 1 (100%) Time: 00:00.123, Memory: 10.00 MB CKEditor5Image Controller (Drupal\Tests\ckeditor5\Unit\CKEditor5ImageController) β Invalid file β β Failed asserting that exception message 'Destination file path is not writable' contains 'The file "foo.txt" exceeds the upload limit defined in your form.'.
So believe this is test coverage works
- πΊπΈUnited States xjm
I still don't think we're quite at a complete RTBC from the above, but I re-reviewed the full details of the private issue and will add my +1 to the RTBC confirming that it does indeed provide test coverage that should prevent the vulnerability from being reintroduced, despite that it is not possible to create an actual full exploit test in this situation. :)