Potential XSS attack with Turbolinks

[mala]

There are potential XSS attack vectors with turbolinks,

• Turbolinks + autolink + file upload
• Turbolinks + open redirect link + XHR level2

Demo: http://mala-mala.sqale.jp/turbolinks_xss/

"restrict to same origin" policy is not enough in some cases.
Some pjax approach framework e.g. jQuery Mobile also contains this problem.

Currently, custom header + cross origin redirect works only with IE10.
However, this is right action based on the specification of CORS.
For the moment, there is no effective method to detecting cross origin redirection.

How to fix

• same origin: check content-type, content-disposition by javascript
• cross origin: don't create open redirector in same origin, we need document for security consideration

[reed]

Thanks for informing us of these issues. For the first one, could we fix it by just removing the hash when checking the link type? Like this:

# current - returns false for /sample.csv#hoge.html
nonHtmlLink = (link) ->
  link.href.match(/\.[a-z]+(\?.*)?$/g) and not link.href.match(/\.html?(\?.*)?$/g)

# fixed - returns true for /sample.csv#hoge.html
nonHtmlLink = (link) ->
  url = removeHash link
  url.match(/\.[a-z]+(\?.*)?$/g) and not url.match(/\.html?(\?.*)?$/g)

This would fix the example you have in your demo, but would it fix all cases?

[mala]

Yeah, check of extension is useful to prejudgment.
Since hash fragment does not affect content-type certainly, removeHash is right.

Some attack vectors will remain.

/sample.csv.html
/sample.csv?dummy.html
/sample.csv/
/some_api.html?format=raw

Wow, I don't want to think for every cases, complicated regexp is meaningless and bad.
So, check of content-type will be required and more important.

OK: text/html, application/xhtml+xml, application/xml
NG: text/plain, text/csv, text/html-sandboxed

[reed]

How does this pull request look?

[mala]

Hmm, text/html-sandboxed was already removed http://www.w3.org/html/wg/wiki/ChangeProposals/text_html_sandboxed
but some another things may be defined as text/html-xxx

"text/html".match(/^(?:text\/html|application\/xhtml\+xml|application\/xml)(?:;|$)/) // match
"text/html-xxxxx; param=text/html".match(/^(?:text\/html|application\/xhtml\+xml|application\/xml)(?:;|$)/) // not match

[reed]

Yeah, nice catch. I updated the PR. Do you think it's sufficient now?

[mala]

Very good, no problem at content-type case.

I can't judge for content-disposition case.
Probably the check of content-type fully covers both.

"Content-Disposition: attachment" should be used together with "application/octet-stream".
The other case is unspecified at RFC2616. It's only based on the specification of the browser.
So, "attachment" file which the user uploaded should not be distributed by text/html.

[reed]

Alright, so I guess we'll just stick with the content-type check for now.

[mala]

I wrote a patch #197

The redirection by the Rails itself can be prevented.
It will not block mod_rewrite or other methods.

[reed]

This looks good. Would you be willing to consider a few cosmetic suggestions?

-  def is_sameorigin(a, b)
+  def same_origin?(a, b)
     a = URI.parse(a)
     b = URI.parse(b)
-    a.scheme + a.host + a.port.to_s == b.scheme + b.host + b.port.to_s
+    [a.scheme, a.host, a.port] == [b.scheme, b.host, b.port]
   end

   def abort_xdomain_redirect
     to_uri = response.headers['Location'] || ""
     current = request.headers['X-XHR-Referer] || ""
-    if (!to_uri.empty? && !current.empty? && !is_sameorigin(current, to_uri))
+    unless to_uri.blank? || current.blank? || same_origin?(current, to_uri)
       self.status = 403
     end
   end

[mala]

Thanks for your help. I just updated pullreqest.

[reed]

I just pulled it in. Thanks again for pointing out these security issues and helping us resolve them.

[frohoff]

It looks like the restriction of using Content-Disposition: attachment only with Content-Type: application/octet-stream set forth by http://tools.ietf.org/html/rfc2616 was lifted in http://tools.ietf.org/html/rfc6266 based on usage in the wild.

According to RFC 2616, the disposition type "attachment" only applies to content of type "application/octet-stream". This restriction has been removed, because recipients in practice do not check the content type, and it also discourages properly declaring the media type.

Applications serving user-uploaded html files and issuing Content-Disposition: attachment and matching Content-Type: text/html headers (as per the updated spec) will still be vulnerable to XSS attacks unless additional checks for the Content-Disposition header in the response are added.