RUBY-783 close security issue in cookie handling

newrelic · Mar 15, 2012 · d910489 · d910489
1 parent 9692df8
commit d910489
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 8 deletions.
diff --git a/lib/new_relic/agent/transaction_info.rb b/lib/new_relic/agent/transaction_info.rb
@@ -1,3 +1,5 @@
+require 'erb'
+
 module NewRelic
   module Agent
     class TransactionInfo
@@ -48,17 +50,22 @@ def self.clear
       def self.reset(request=nil)
         clear
         instance = get
+        instance.token = get_token(request)
+      end
 
-        if request
-          agent_flag = request.cookies['NRAGENT']
-          if agent_flag
-            s = agent_flag.split("=")
-            if s.length == 2
-              if s[0] == "tk" && s[1]
-                instance.token = s[1]
-              end
+      def self.get_token(request)
+        return nil unless request
+
+        agent_flag = request.cookies['NRAGENT']
+        if agent_flag
+          s = agent_flag.split("=")
+          if s.length == 2
+            if s[0] == "tk" && s[1]
+              ERB::Util.h(s[1])
             end
           end
+        else
+          nil
         end
       end
     end

diff --git a/test/new_relic/agent/transaction_info_test.rb b/test/new_relic/agent/transaction_info_test.rb
@@ -0,0 +1,13 @@
+require File.expand_path(File.join(File.dirname(__FILE__),'..','..','test_helper'))
+require 'ostruct'
+
+class NewRelic::Agent::TransactionInfoTest < Test::Unit::TestCase
+  def setup
+    @request = OpenStruct.new(:cookies => {'NRAGENT' => 'tk=1234<tag>evil</tag>5678'})
+  end
+
+  def test_get_token_gets_sanitized_token_from_cookie
+    assert_equal('1234&lt;tag&gt;evil&lt;/tag&gt;5678',
+                 NewRelic::Agent::TransactionInfo.get_token(@request))
+  end
+end