#1455 closed defect (fixed)

WUI: ambiently accessible pages should framebust in order to prevent UI redressing attacks

Reported by: davidsarah Owned by:
Priority: normal Milestone: 1.13.0
Component: code-frontend-web Version: 1.8.2
Keywords: security ambient wui redressing review-needed Cc:
Launchpad Bug:

Description (last modified by ChosenOne)

If an ambiently accessible WUI page (one that does not require a capability to access, such as the Welcome page) is loaded in a frame or iframe, the loading frame might be able to perform some UI redressing or clickjacking attacks.

For example, the loading frame could entice the user to click the "Create a directory" button, when it should not have the authority to create a directory on that grid.

This is not a very strong attack. In any case, it can be prevented by including some framebusting code on ambiently accessible WUI pages (or all WUI pages).

Change History (16)

comment:1 Changed at 2011-07-31T04:36:13Z by davidsarah

How not to framebust: http://seclab.stanford.edu/websec/framebusting/framebust.pdf

The way to do it securely seems to be to send an X-Frame-Options: DENY header.

comment:2 Changed at 2012-08-28T18:11:05Z by davidsarah

According to https://developer.mozilla.org/en-US/docs/The_X-FRAME-OPTIONS_response_header , X-Frame-Options is supported by:

  • Internet Explorer 8.0
  • Firefox 3.6.9 (Gecko 1.9.2.9)
  • Opera 10.50
  • Safari 4.0
  • Chrome 4.1.249.1042

I think there might be some benefit in including the header for all WUI pages, not just ambiently accessible ones. In conjunction with #1797, that would simplify reasoning about some of the attacks we were worried about in the 2012-08-28 conference call.

comment:3 Changed at 2013-11-30T14:29:16Z by ChosenOne

  • Description modified (diff)

I am trying to take this, but I have serious problems finding the appropriate code to patch. It looks like a lot of HTTP handling is scattered through all files in src/web/, which makes it particularly hard to add a security header for all HTTP responses :(

It is unclear to me what the childFactory methods in web/root.py do and the nevow guides weren't very helpful. If they are called for each incoming request, I would consider writing a function that takes a context (ctx) and sets the desired headers for the corresponding request (IRequest(ctx) it seems).

Otherwise I would have to patch the request handlers for every HTTP method (i.e. render_FOO). A pointer for this would also be helpful.

If it's not explicitly undesired, I am considering adding other security headers in a follow-up and might therefore do some changes that are not exactly required for the X-Frame-Options patch.

Oh, and on a side-note: I would suggest to set the X-Frame-Options header to SAMEORIGIN and not DENY. This way, HTML files hosted on tahoe can be framed by other files in the grid. I assume that some users might require this. If there is consensus to take DENY, I will happily obey ;-)

comment:4 Changed at 2013-11-30T15:25:19Z by ChosenOne

OK, it seems easy enough to do this for uri/ and file/ resources by patching FileHandler and URIHandler classes in root.py. See this commit on my feature branch

I have yet to figure out how to do this for things like /statistics, /status, etc. It seems undesirable to re-implement this for every subpage in its own file in src/web/. This would mean that every new subresource would have to remember doing this again to ensure that no URL is left behind ;)

Wouldn't it make sense to build a tiny abstraction layer on top of nevow's rend.Page that ensures proper response headers and is to be used in all files? This would still require changes in all files, but these could be less careful, as we just replace rend.Page references.

comment:5 Changed at 2013-11-30T22:37:10Z by ChosenOne

Ha, I think I solved it differently. Please see https://github.com/tahoe-lafs/tahoe-lafs/pull/72 or https://github.com/freddyb/tahoe-lafs/commits/ticket1455-x-frame-options

I think warner did the most WUI things, so review appreciated

comment:6 Changed at 2013-12-01T12:40:19Z by daira

  • Keywords review-needed added
  • Owner set to daira
  • Priority changed from minor to normal
  • Status changed from new to assigned

Reviewing.

comment:7 Changed at 2015-01-29T19:51:50Z by daira

  • Milestone changed from undecided to 1.11.0

comment:8 Changed at 2015-02-09T01:57:42Z by daira

I seem to have lost track of this after saying I'd review it. I'll rebase it soon.

comment:9 Changed at 2015-02-09T02:31:57Z by daira

  • Keywords test-needed added; review-needed removed

Rebased to https://github.com/tahoe-lafs/tahoe-lafs/commits/1455.x-frame-options.1. (I changed SAMEORIGIN to DENY.)

Needs a test.

Last edited at 2015-02-09T02:32:20Z by daira (previous) (diff)

comment:10 Changed at 2015-02-09T02:34:20Z by daira

BTW, it needs to be DENY rather than SAMEORIGIN because the attacking page might also be on the grid.

comment:11 Changed at 2016-03-22T05:02:52Z by warner

  • Milestone changed from 1.11.0 to 1.12.0

Milestone renamed

comment:12 Changed at 2016-06-28T18:20:37Z by warner

  • Milestone changed from 1.12.0 to 1.13.0

moving most tickets from 1.12 to 1.13 so we can release 1.12 with magic-folders

comment:13 Changed at 2018-04-13T17:15:57Z by exarkun

  • Keywords review-needed added; test-needed removed
  • Owner daira deleted
  • Status changed from assigned to new

comment:14 Changed at 2018-05-28T12:12:41Z by Jean-Paul Calderone <exarkun@…>

In 718fa44/trunk:

Add "X-Frame-Options: DENY" header to all pages. refs #1455

Signed-off-by: Daira Hopwood <daira@…>

comment:15 Changed at 2018-05-28T12:16:14Z by exarkun

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.