Comment 20 for bug 1593799

Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Thanks, Travis. Sorry to be so long getting to this.

I hate to be pedantic (but I used to be a professor, so I can't help it). Two comments:

(1) The scenario in your "Summary" isn't quite right. For that exact scenario to work you'd also have to have one of the following:

(A) Mallory has permission to "publicize" images (limited to admins by default policy). Otherwise, Alice can't access the "new" image X uploaded by Mallory.

(B) Mallory doesn't have permission to "publicize" images, but knows Alice's project ID, and shares the image with Alice. (If we assume that the original image X was shared by Bob, since Mallory didn't own the original image X, she couldn't see the full member list of everyone Bob had shared it with, so she'd have to guess or do some social engineering.)

I guess what I'm really getting at here is that the most likely scenario for this to work is that either Bob and Mallory are the same person or are in cahoots. But after writing all that down, in the long run, it doesn't matter, it *is* possible for this to happen, so maybe your summary is sufficient after all.

(2) If a deployer wants to purge the database for space reasons, it would be OK to manually remove the soft-deleted rows from the image_properties, image_tags, image_members, and image_locations tables. (You do say "don't delete rows from the images table", but maybe it's worth saying that it's OK to delete rows from these other tables.) I haven't read a lot of security notes, so ignore this suggestion if it's a dumb idea or inconsistent with the usual practice.

Otherwise, the note LGTM.