Permalink
Browse files

Clean JSONP callback

* strips any non-word characters: attemps to ensure only valid JavaScript
  function names are returned
* reduces XSS vulnerability
  • Loading branch information...
1 parent 759ab41 commit 67d5660bb0b48b693ff95d0572607894d310dbad @mtodd mtodd committed Apr 6, 2011
Showing with 44 additions and 1 deletion.
  1. +9 −1 lib/rack/contrib/jsonp.rb
  2. +35 −0 test/spec_rack_jsonp.rb
View
10 lib/rack/contrib/jsonp.rb
@@ -57,7 +57,15 @@ def has_callback?(request)
#
def pad(callback, response, body = "")
response.each{ |s| body << s.to_s }
- ["#{callback}(#{body})"]
+ ["#{clean(callback)}(#{body})"]
+ end
+
+ # Removes any non-valid characters for a JavaScript function name.
+ #
+ # clean("foo<script>alert(1);</script>") #=> "fooscriptalert1script"
+ #
+ def clean(callback)
+ callback.gsub(/\W+/, '')
end
end
View
35 test/spec_rack_jsonp.rb
@@ -52,6 +52,41 @@
headers['Content-Type'].should.equal('application/javascript')
end
+ context "with XSS vulnerability attempts" do
+ specify "should clean the callback to include only valid characters for a JavaScript function name" do
+ test_body = '{"bar":"foo"}'
+ callback = 'foo<bar>baz()'
+ callback_cleaned = 'foobarbaz'
+ app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ body = Rack::JSONP.new(app).call(request).last
+ body.should.not.include "<script>"
+ body.should.equal ["#{callback_cleaned}(#{test_body})"]
+ end
+
+ specify "should not include <script> tags in the callback" do
+ test_body = '{"bar":"foo"}'
+ callback = 'foo<script>alert(1)</script>'
+ callback_cleaned = 'fooscriptalert1script'
+ app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ body = Rack::JSONP.new(app).call(request).last
+ body.should.not.include "<script>"
+ body.should.equal ["#{callback_cleaned}(#{test_body})"]
+ end
+
+ specify "should not include multiple statements" do
+ test_body = '{"bar":"foo"}'
+ callback = 'foo%3balert(1)//'
+ callback_cleaned = 'fooalert1'
+ app = lambda { |env| [200, {'Content-Type' => 'application/json'}, [test_body]] }
+ request = Rack::MockRequest.env_for("/", :params => "foo=bar&callback=#{callback}")
+ body = Rack::JSONP.new(app).call(request).last
+ body.should.not.include "<script>"
+ body.should.equal ["#{callback_cleaned}(#{test_body})"]
+ end
+ end
+
end
specify "should not change anything if no callback param is provided" do

0 comments on commit 67d5660

Please sign in to comment.