From 3eaa9272b12aa2c568f7595b99bf7aa4a62ff92f Mon Sep 17 00:00:00 2001
From: Andrew Cantino <cantino@gmail.com>
Date: Sat, 7 Sep 2013 00:34:51 -0700
Subject: [PATCH] Prevent some possible XSS attacks where user input makes its
 way into JSON and contains </script> tags.

---
 .../peak_detector_agent/_show.html.erb        |  6 ++---
 .../twitter_stream_agent/_show.html.erb       |  4 ++--
 app/views/agents/show.html.erb                |  4 ++--
 app/views/events/show.html.erb                |  2 +-
 lib/utils.rb                                  | 16 ++++++++++++--
 spec/lib/utils_spec.rb                        | 22 +++++++++++++++++++
 6 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/app/views/agents/agent_views/peak_detector_agent/_show.html.erb b/app/views/agents/agent_views/peak_detector_agent/_show.html.erb
index 29cb8921..cc9b341c 100644
--- a/app/views/agents/agent_views/peak_detector_agent/_show.html.erb
+++ b/app/views/agents/agent_views/peak_detector_agent/_show.html.erb
@@ -17,9 +17,9 @@
       <script>
         $(function() {
           var $chart = $(".chart-container.group-<%= index.to_s %>").last();
-          var data = <%= data.map {|count, time| { :x => time.to_i, :y => count.to_i } }.to_json.html_safe %>;
-          var peaks = <%= ((@agent.memory[:peaks] && @agent.memory[:peaks][group_name]) || []).to_json.html_safe %>;
-          var name = <%= group_name.to_json.html_safe %>;
+          var data = <%= Utils.jsonify(data.map {|count, time| { :x => time.to_i, :y => count.to_i } }) %>;
+          var peaks = <%= Utils.jsonify((@agent.memory[:peaks] && @agent.memory[:peaks][group_name]) || []) %>;
+          var name = <%= Utils.jsonify(group_name) %>;
 
           renderGraph($chart, data, peaks, name);
         });
diff --git a/app/views/agents/agent_views/twitter_stream_agent/_show.html.erb b/app/views/agents/agent_views/twitter_stream_agent/_show.html.erb
index e2f28f24..d1bdc5b0 100644
--- a/app/views/agents/agent_views/twitter_stream_agent/_show.html.erb
+++ b/app/views/agents/agent_views/twitter_stream_agent/_show.html.erb
@@ -35,8 +35,8 @@
         <script>
           $(function() {
             var $chart = $(".chart-container.group-<%= index.to_s %>").last();
-            var data = <%= group.select {|e| e.payload[:count].present? }.sort_by {|e| e.payload[:time] }.map {|e| { :x => e.payload[:time].to_i, :y => e.payload[:count].to_i } }.to_json.html_safe %>;
-            var name = <%= filter.to_json.html_safe %>;
+            var data = <%= Utils.jsonify(group.select {|e| e.payload[:count].present? }.sort_by {|e| e.payload[:time] }.map {|e| { :x => e.payload[:time].to_i, :y => e.payload[:count].to_i } }) %>;
+            var name = <%= Utils.jsonify(filter) %>;
 
             renderGraph($chart, data, [], name);
           });
diff --git a/app/views/agents/show.html.erb b/app/views/agents/show.html.erb
index 6c20ed1f..a07d03a4 100644
--- a/app/views/agents/show.html.erb
+++ b/app/views/agents/show.html.erb
@@ -132,12 +132,12 @@
 
             <p>
               <b>Options:</b>
-              <pre><%= JSON.pretty_generate @agent.options || {} %></pre>
+              <pre><%= Utils.pretty_jsonify @agent.options || {} %></pre>
             </p>
 
             <p>
               <b>Memory:</b>
-              <pre><%= JSON.pretty_generate @agent.memory || {} %></pre>
+              <pre><%= Utils.pretty_jsonify @agent.memory || {} %></pre>
             </p>
           </div>
         </div>
diff --git a/app/views/events/show.html.erb b/app/views/events/show.html.erb
index 6c15b037..4351a455 100644
--- a/app/views/events/show.html.erb
+++ b/app/views/events/show.html.erb
@@ -7,7 +7,7 @@
 
       <p>
         <b>Payload:</b>
-        <pre><%= JSON.pretty_generate @event.payload || {} %></pre>
+        <pre><%= Utils.pretty_jsonify @event.payload || {} %></pre>
       </p>
 
       <% if @event.lat && @event.lng %>
diff --git a/lib/utils.rb b/lib/utils.rb
index 2c52008c..258e5267 100644
--- a/lib/utils.rb
+++ b/lib/utils.rb
@@ -52,7 +52,19 @@ module Utils
     end
   end
 
-  def self.jsonify(thing)
-    thing.to_json.gsub('</', '<\/').html_safe
+  # Output JSON that is ready for inclusion into HTML.  If you simply use to_json on an object, the
+  # presence of </script> in the valid JSON can break the page and allow XSS attacks.
+  # Optionally, pass `:skip_safe => true` to not call html_safe on the output.
+  def self.jsonify(thing, options = {})
+    json = thing.to_json.gsub('</', '<\/')
+    if !options[:skip_safe]
+      json.html_safe
+    else
+      json
+    end
+  end
+
+  def self.pretty_jsonify(thing)
+    JSON.pretty_generate(thing).gsub('</', '<\/')
   end
 end
\ No newline at end of file
diff --git a/spec/lib/utils_spec.rb b/spec/lib/utils_spec.rb
index 57de0def..4368916c 100644
--- a/spec/lib/utils_spec.rb
+++ b/spec/lib/utils_spec.rb
@@ -55,4 +55,26 @@ describe Utils do
       Utils.values_at({ :foo => { :bar => "escape this!?" }}, "escape $.foo.bar").should == ["escape+this%21%3F"]
     end
   end
+
+  describe "#jsonify" do
+    it "escapes </script> tags in the output JSON" do
+      cleaned_json = Utils.jsonify(:foo => "bar", :xss => "</script><script>alert('oh no!')</script>")
+      cleaned_json.should_not include("</script>")
+      cleaned_json.should include("<\\/script>")
+    end
+
+    it "html_safes the output unless :skip_safe is passed in" do
+      Utils.jsonify({:foo => "bar"}).should be_html_safe
+      Utils.jsonify({:foo => "bar"}, :skip_safe => false).should be_html_safe
+      Utils.jsonify({:foo => "bar"}, :skip_safe => true).should_not be_html_safe
+    end
+  end
+
+  describe "#pretty_jsonify" do
+    it "escapes </script> tags in the output JSON" do
+      cleaned_json = Utils.pretty_jsonify(:foo => "bar", :xss => "</script><script>alert('oh no!')</script>")
+      cleaned_json.should_not include("</script>")
+      cleaned_json.should include("<\\/script>")
+    end
+  end
 end
\ No newline at end of file