#4277 Attachments of content-type image/* should be displayed inline

v1.0.0
closed
General
Cory Johns
2015-08-20
2012-05-24
No

It is safe to show image attachments directly, rather than serving them as downloads (like we must do for everything else, for security)

Discussion

  • Dave Brondsema

    Dave Brondsema - 2012-06-01
    • size: --> 2
     
  • Dave Brondsema

    Dave Brondsema - 2012-06-07
    • labels: --> v2
     
  • Jenny Steele

    Jenny Steele - 2012-06-18
    • status: open --> in-progress
    • assigned_to: Jenny Steele
     
  • Jenny Steele

    Jenny Steele - 2012-06-18

    On allura js/4277. To test, attach both an image and a non-image file to an artifact (should work for tickets, wiki pages, discussion posts, etc the same). Clicking the image should display the image full-size, but clicking the non-image will prompt a download.

     
  • Jenny Steele

    Jenny Steele - 2012-06-18
    • status: in-progress --> code-review
    • qa: Cory Johns
     
  • Cory Johns

    Cory Johns - 2012-06-20
    • status: code-review --> closed
     
  • Dave Brondsema

    Dave Brondsema - 2012-06-25
    • status: closed --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2012-06-25

    This isn't secure enough. If I upload an HTML attachment and then (directly in mongo) update the content type to text/html/image/ then the attachment will be displayed inline, and Firefox does show it as regular HTML. This is possible since we currently trust user-provided content types.

    • we should only do inline display of content types that start with image/
    • we should have a new ticket to not trust user-provided content types at all (the attach() method calls, and mail_tasks.py / handle_message() )
     
  • Dave Brondsema

    Dave Brondsema - 2012-06-25

    Actually that is not the problem (although I think it's still safer to do a startswith check). The problem is that self.content_type is used to to the check, but serve() in filesystem.py uses the GridFS fp content_type. We need to check the same value. Offhand, I'm not sure which is best.

     
  • Jenny Steele

    Jenny Steele - 2012-06-26
    • status: in-progress --> code-review
     
  • Jenny Steele

    Jenny Steele - 2012-06-26

    I made them both use self.content_type.

     
  • Cory Johns

    Cory Johns - 2012-06-27
    • status: code-review --> closed
     

Log in to post a comment.