Page MenuHomePhabricator

Security review of the ImageTweaks extension ahead of production deployment
Closed, ResolvedPublic

Description

Project Information

Description of the tool/project

ImageTweaks is a new extensions developed by WMF engineers in the Multimedia team to provide on-wiki media editing tools. Still under development, but likely most of the code will be (client-side) JavaScript image file reading, manipulation and writing for then uploading through one of the usual tools into the upload pipeline for Commons.

Description of how the tool will be used at WMF

Production deployment of the extension at first for logged-in users on Commons for file manipulation, initially as a Beta Feature, then for all users. Plausibly, later for all users for in-article non-destructive image modification without needing to upload near-duplicate versions of files.

Dependencies

List dependencies, or upstream projects that this project relies on

Has this project been reviewed before?

please link to tasks or wiki pages of previous reviews
Nope.

Working test environment

please link or describe setup process for setting up a test environment

Post-deployment

name of team responsible for tool/project after deployment and primary contact
Multimedia still. :-)

Event Timeline

@csteipp we just spoke about the extension's current state, and though it's not going to be deployed pending Thumbor deployment and configuration for our use, we are confident that the current state of the codebase includes the lion's share of the code that will be present upon deployment, so we're comfortable asking for the security review now. Thanks!

General Observations

Excellent developer documentation in client-side code.

Issues

No issues found.

Files

./ApiImageTweaks.php

OK

./extension.json

OK

./ImageTweaks.hooks.php

OK

./ImageTweaks.php

OK

./includes/HTMLImageDisplayField.php

NOTE
- line 68, TODO item for server-side validation

./includes/UploadFromLocalFile.php

OK

./includes/UploadFromRequest.php

OK

./node_modules/caman-dist-only/dist/caman.full.js

OK (partially reviewed; DOM XSS sinks and file I/O only)

./resources/lib/ExifRestorer.js

OK

./resources/src/caman.fix.js

NOTE
- ensure that when upstream fix lands, this code is removed and the dependency is updated

./resources/src/caman.flip.js

OK

./resources/src/imageeditor.js

OK

./resources/src/imagetool.js

OK

./resources/src/imagetweaks.bootstrap.js

OK
MarkTraceur raised the priority of this task from Medium to High.Dec 5 2016, 10:04 PM
MarkTraceur moved this task from Untriaged to Next up on the Multimedia board.
MarkTraceur raised the priority of this task from High to Needs Triage.Dec 6 2016, 5:55 PM
MarkTraceur triaged this task as Medium priority.
MarkTraceur moved this task from Next up to Tracking on the Multimedia board.
Bawolff subscribed.

As far as I understand, there is nothing more to do on this bug, so I'm closing resolved.