Apply some changes to JSON-P responses to mitigate the Rosetta Flash …

…attack.

For more info see http://miki.it/blog/2014/7/8/abusing-jsonp-with-rosetta-flash/

javascript/adapters/node_adapter.js

@@ -22,6 +22,8 @@ Faye.NodeAdapter = Faye.Class({
   TYPE_SCRIPT:  {'Content-Type': 'text/javascript; charset=utf-8'},
   TYPE_TEXT:    {'Content-Type': 'text/plain; charset=utf-8'},
 
+  VALID_JSONP_CALLBACK: /^[a-z_\$][a-z0-9_\$]*(\.[a-z_\$][a-z0-9_\$]*)*$/i,
+
   initialize: function(options) {
     this._options    = options || {};
     this._endpoint   = this._options.mount || this.DEFAULT_ENDPOINT;
@@ -135,12 +137,21 @@ Faye.NodeAdapter = Faye.Class({
           headers = Faye.extend({}, type),
           origin  = request.headers.origin;
 
+      if (!this.VALID_JSONP_CALLBACK.test(jsonp))
+        return this._returnError(response, {message: 'Invalid JSON-P callback: ' + jsonp});
+
       if (origin) headers['Access-Control-Allow-Origin'] = origin;
       headers['Cache-Control'] = 'no-cache, no-store';
+      headers['X-Content-Type-Options'] = 'nosniff';
 
       this._server.process(message, request, function(replies) {
         var body = Faye.toJSON(replies);
-        if (isGet) body = jsonp + '(' + this._jsonpEscape(body) + ');';
+
+        if (isGet) {
+          body = '/**/' + jsonp + '(' + this._jsonpEscape(body) + ');';
+          headers['Content-Disposition'] = 'attachment; filename=f.txt';
+        }
+
         headers['Content-Length'] = new Buffer(body, 'utf8').length.toString();
         headers['Connection'] = 'close';
 

lib/faye/adapters/rack_adapter.rb

@@ -15,6 +15,8 @@ class RackAdapter
     TYPE_SCRIPT = {'Content-Type' => 'text/javascript; charset=utf-8'}
     TYPE_TEXT   = {'Content-Type' => 'text/plain; charset=utf-8'}
 
+    VALID_JSONP_CALLBACK = /^[a-z_\$][a-z0-9_\$]*(\.[a-z_\$][a-z0-9_\$]*)*$/i
+
     # This header is passed by Rack::Proxy during testing. Rack::Proxy seems to
     # set content-length for you, and setting it in here really slows the tests
     # down. Better suggestions welcome.
@@ -94,22 +96,33 @@ def handle_request(request)
 
       debug("Received message via HTTP #{request.request_method}: ?", json_msg)
 
-      message = MultiJson.load(json_msg)
-      request.env['rack.hijack'].call if request.env['rack.hijack']
-
+      message  = MultiJson.load(json_msg)
       jsonp    = request.params['jsonp'] || JSONP_CALLBACK
       headers  = request.get? ? TYPE_SCRIPT.dup : TYPE_JSON.dup
       origin   = request.env['HTTP_ORIGIN']
       callback = request.env['async.callback']
-      hijack   = request.env['rack.hijack_io']
+
+      if jsonp !~ VALID_JSONP_CALLBACK
+        error 'Invalid JSON-P callback: ?', jsonp
+        return [400, TYPE_TEXT, ['Bad request']]
+      end
 
       headers['Access-Control-Allow-Origin'] = origin if origin
       headers['Cache-Control'] = 'no-cache, no-store'
+      headers['X-Content-Type-Options'] = 'nosniff'
+
+      request.env['rack.hijack'].call if request.env['rack.hijack']
+      hijack = request.env['rack.hijack_io']
 
       EventMachine.next_tick do
         @server.process(message, request) do |replies|
           response = Faye.to_json(replies)
-          response = "#{ jsonp }(#{ jsonp_escape(response) });" if request.get?
+
+          if request.get?
+            response = "/**/#{ jsonp }(#{ jsonp_escape(response) });"
+            headers['Content-Disposition'] = 'attachment; filename=f.txt'
+          end
+
           headers['Content-Length'] = response.bytesize.to_s unless request.env[HTTP_X_NO_CONTENT_LENGTH]
           headers['Connection'] = 'close'
           debug 'HTTP response: ?', response

spec/javascript/node_adapter_spec.js

@@ -246,8 +246,8 @@ JS.ENV.NodeAdapterSpec = JS.Test.describe("NodeAdapter", function() { with(this)
         get("/bayeux", params)
         check_status(200)
         check_content_type("text/javascript")
-        check_content_length("42")
-        check_body('callback([{"channel":"/meta/handshake"}]);')
+        check_content_length("46")
+        check_body('/**/callback([{"channel":"/meta/handshake"}]);')
       }})
 
       it("does not let the client cache the response", function() { with(this) {
@@ -267,8 +267,8 @@ JS.ENV.NodeAdapterSpec = JS.Test.describe("NodeAdapter", function() { with(this)
         get("/bayeux", params)
         check_status(200)
         check_content_type("text/javascript")
-        check_content_length("47")
-        check_body('jsonpcallback([{"channel":"/meta/handshake"}]);')
+        check_content_length("51")
+        check_body('/**/jsonpcallback([{"channel":"/meta/handshake"}]);')
       }})
     }})
 
@@ -292,6 +292,13 @@ JS.ENV.NodeAdapterSpec = JS.Test.describe("NodeAdapter", function() { with(this)
       behavesLike("bad GET request")
     }})
 
+    describe("with a callback that's not a JS identifier", function() { with(this) {
+      before(function() { with(this) {
+        params.jsonp = "42"
+      }})
+      behavesLike("bad GET request")
+    }})
+
     describe("missing message", function() { with(this) {
       before(function() { with(this) {
         delete params.message

spec/ruby/rack_adapter_spec.rb

@@ -160,8 +160,8 @@
         get "/bayeux", params
         status.should == 200
         content_type.should == "text/javascript; charset=utf-8"
-        content_length.should == "42"
-        body.should == 'callback([{"channel":"/meta/handshake"}]);'
+        content_length.should == "46"
+        body.should == '/**/callback([{"channel":"/meta/handshake"}]);'
       end
 
       it "does not let the client cache the response" do
@@ -190,8 +190,8 @@
         get "/bayeux", params
         status.should == 200
         content_type.should == "text/javascript; charset=utf-8"
-        content_length.should == "47"
-        body.should == 'jsonpcallback([{"channel":"/meta/handshake"}]);'
+        content_length.should == "51"
+        body.should == '/**/jsonpcallback([{"channel":"/meta/handshake"}]);'
       end
     end
 
@@ -213,6 +213,11 @@
       it_should_behave_like "bad GET request"
     end
 
+    describe "with a callback that's not a JS identifier" do
+      before { params[:jsonp] = "42" }
+      it_should_behave_like "bad GET request"
+    end
+
     describe "missing message" do
       before { params.delete(:message) }
       it_should_behave_like "bad GET request"