449440 - Cannot remove uploaded images

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Chris Ilias [:cilias]]

Sometimes spammers use the image uploader. I can reject the edit, but the image is still available via direct URL. I've used the [Remove] button at <http://support.mozilla.com/tiki-admin.php?locale=en-US&page=gal>, but it did not remove <http://support.mozilla.com/img/wiki_up/P3140331.JPG>.

Comment 1

[Chris Ilias [:cilias]]

http://support.mozilla.com/en-US/kb/img/wiki_up/21e98ab87f28793d68c9e7ef0d6d547b.jpg

Comment 2

[Chris Ilias [:cilias]]

http://support.mozilla.com/img/wiki_up/mee%27%5D.jpg

Comment 3

[Chris Ilias [:cilias]]

Looking at the user profile of the user who uploaded the last image in comment 2 <http://support.mozilla.com/tiki-adminusers.php?offset=0&numrows=10&sort_mode=login_asc&user=33927#2>, time of registration and last login are a couple of minutes apart. Looking at the action log, uploading that image is the only thing that user did.

Right now, it looks like the system is purposely being abused.

Comment 4

[Laura Thomson :laura]

Can we please set things up so we only allow embedding of images when the referer is also on SUMO?  (When this is done please assign this bug back to webdev so we can build a UI for image deletion as well)

Comment 5

[Jeremy Orem [:oremj]]

+
+  RewriteCond %{HTTP_REFERER} !^$
+  RewriteCond %{HTTP_REFERER} !^https?://support.mozilla.com/.* [NC]
+  RewriteRule ^/.*/img/wiki_up/.* - [F]
+

This works for the most part.  The image will be available if it ends up in the NS cache.

Comment 6

[Laura Thomson :laura]

The changes to rewrites move this down from critical (yay, thanks oremj). Still need UI for deletions though.  

Comment 7

[Laura Thomson :laura]

UI to be done in 0.6.4

Comment 8

[Chris Ilias [:cilias]]

<http://support.mozilla.com/en-US/kb/img/wiki_up/743.jpg>

Comment 9

[Chris Ilias [:cilias]]

http://support.mozilla.com/en-US/kb/img/wiki_up/saltillo.jpg

Comment 10

[Les Orchard [:lorchard]]

Grabbing this one from Laura - looks like it might be a bit of a pain to fix, but I might have a lead on it.

Comment 11

[Les Orchard [:lorchard]]

Attachment: patch (-p1) that sweeps for unused uploaded images that don't have gallery DB entries

The original admin code seems to expect images to be tracked in an tiki_images table, but the code for uploading images over in tiki-editpage.php never touches that table.  Thus, the remove button did nothing for these images.

I think the attached patch should work to sweep up images that have been uploaded via tiki-editpage.php yet not included in the text of any pages.  I'm a *little* leary about it, since it does delete and I'm wondering if it should back up things anywhere.  Seems weird that the original code doesn't do this, so I'm wondering also if this is the right approach.

Comment 12

[Les Orchard [:lorchard]]

Another thought: To make this less sweeping and less dangerous to the site overall, I wonder if it would be useful to make this available as a per-page image removal admin action, using the page hash prefix introduced with bug 450548 to constrain deletion to a less than site-wide scope.  Of course, that will also limit it to images uploaded by the patch in bug 450548, but it's an idea

Comment 13

[Les Orchard [:lorchard]]

in trunk as r20631 and in prod as r20632

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Hey Les: how do I test this on support-stage?

Comment 15

[Les Orchard [:lorchard]]

The patch deletes images that have been uploaded, but are no longer referenced in the source of any pages.  If you have admin access on support-stage (unlike me), the following should exercise the code:

* Edit a page, eg:

http://support-stage.mozilla.org/tiki-editpage.php?locale=en-US&page=*Installing+Firefox+on+Mac

* Select one or more images for upload under "Upload Images" in the editing form.  As you select those images, you should see the future URLs for those images appear as markup in the textarea with the page source.

* Hit "preview" to actually upload the images.  After the page reloads, you should see your uploaded images in the preview where the markup was inserted.

* Go into the textarea and cut the markup for the images you just uploaded.  You should paste this cut markup into a textedit window, because you'll need those image URLs in a second. 

* Hit "preview" again, and the images should no longer be visible in the page.

* Now, just to be sure, visit the URLs of the uploaded images that have been removed from the page markup.  They should not yet be 404s.

* Visit the admin page for image management:

http://support-stage.mozilla.org/tiki-admin.php?locale=en-US&page=gal

* Scroll down to the "Exterminator" section, hit the "Remove" button.

* Try visiting those image URLs again, and they should now come up as 404s.

Comment 16

[Les Orchard [:lorchard]]

Oh, and to be absolutely certain that nothing that shouldn't get deleted goes away, try uploading some images and leave them in the markup.  Hit the "remove" button in the admin page, and verify that the images still used in page markup are still accessible.

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot, post-fix, step one--image uploaded

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot, post-fix, step two--image removed

Comment 19

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using the immaculate STR helpfully provided by Les in comment 15; you are to be commended, sire.

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

(Oh, and I tested comment 16, too.)

Comment 21

[James Socol [:jsocol, :james]]

Not sure why tiki_discuss was added here.

Comment 22

[Louis-Philippe Huberdeau]

Probably related to an other issue. Tiki now uses file galleries to store attachments by default, so this bug is obsolete. However, there might be some planification required for the migration.

Comment 23

[James Socol [:jsocol, :james]]

Based on comment 22 -> tiki_test. Test this on tiki-trunk.m.c and potentially open a new bug.

Comment 24

[Will Kahn-Greene [:willkg] ET needinfo? me]

These bugs are all resolved, so I'm removing the security flag from them.