Page MenuHomePhabricator

Security review for Extension:DeleteBatch
Closed, DeclinedPublic


Extension has been requested at T108501: Activate Extension:DeleteBatch and Extension:UndeleteBatch on en.Wikisource, but has not been yet reviewed.

Event Timeline

Besides the fact that the code is old and somewhat horrible (trust me, I worked on it ;-), security-wise there are at least two things to consider:

  • The special page allows deleting pages either as yourself or as the system account "Delete page script". When choosing the latter option, there's no log or anything about who really chose to delete a bunch of pages.
  • The foreach loop in DeleteBatchForm::showForm() looks dubious.
Legoktm changed the task status from Open to Stalled.Sep 18 2016, 6:38 PM
Legoktm added a subscriber: Legoktm.

I don't think this extension is ready for a security review yet. It need to be modernized (e.g. use HTMLForm, extension.json, etc.) first. Also, I'm not sold on having 2 extensions that do very similar things: Nuke and DeleteBatch. I'd rather see their functionality merged, which basically means fixing T33858: Make Extension:Nuke work with edits that aren't recent.

I agree. Just saw the requests to have this installed and created the task for when it is needed. But if the extension code is horrible as @ashley said, maybe it is not even worth installing this but expanding the capabilities of Nuke.

@Legoktm, @MarcoAurelio can you give an update on the status of the extension? Is it ready to review now? If not, I say we close this ticket as invalid and create another at a later date should the module prove ready for review and likely to be deployed.

@Legoktm, @MarcoAurelio: Could you answer the last comment please / decide? Thanks!

I'm not sure how I can provide any answer. I am not a coder, developer or mantainer of the extension.

Bawolff added a subscriber: Bawolff.

So quick security look, its probably fine. My biggest concern is the total absolute lack of limits. You could literally use this extension to delete every page on the wiki in one go. I'm not sure if that's functionality we want to expose to the user.

However as @Legoktm pointed out, it doesn't follow modern MediaWiki coding conventions. It would also be nice to clarify the usecase.

So next steps to get this extension deployed, if there is still interest:

  • Modernize the code to follow recent mediawiki coding conventions, enable phpcs, etc
  • Clarify what the use case is, how it differs from other extensions like Nuke, and that it makes sense to have a separate extension.
  • Verify users still want this.
  • Some thought has to be put in to what appropriate limits are. I don't think allowing all pages in a wiki to be deleted in one go is sane (for our usecase. It could make total sense for someone else's use case).

To that end, I'm marking this declined for now. If the above is done, please reopen.