Merge pull request #188 from Shopify/secure-hmac-signature

Escape ampersand in params or equal in keys before signing with hmac.

lib/shopify_api/session.rb

@@ -1,4 +1,5 @@
 require 'openssl'
+require 'rack'
 
 module ShopifyAPI
 
@@ -53,8 +54,16 @@ module ShopifyAPI
         params = params.with_indifferent_access
         return false unless signature = params[:hmac]
 
-        sorted_params = params.except(:signature, :hmac, :action, :controller).collect{|k,v|"#{k}=#{v}"}.sort.join('&')
-        OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new(), secret, sorted_params) == signature
+        calculated_signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new(), secret, encoded_params_for_signature(params))
+
+        Rack::Utils.secure_compare(calculated_signature, signature)
+      end
+
+      private
+
+      def encoded_params_for_signature(params)
+        params = params.except(:signature, :hmac, :action, :controller)
+        params.map{|k,v| "#{URI.escape(k.to_s, '&=%')}=#{URI.escape(v.to_s, '&%')}"}.sort.join('&')
       end
     end
 

shopify_api.gemspec

@@ -25,6 +25,7 @@ Gem::Specification.new do |s|
   s.license = 'MIT'
 
   s.add_dependency("activeresource")
+  s.add_dependency("rack")
 
   dev_dependencies = [['mocha', '>= 0.9.8'],
                       ['fakeweb'], 

test/session_test.rb

@@ -171,6 +171,24 @@ class SessionTest < Test::Unit::TestCase
       assert_equal true, ShopifyAPI::Session.validate_signature(params)
     end
 
+    should "return true when validating signature of params with ampersand and equal sign characters" do
+      ShopifyAPI::Session.secret = 'secret'
+      params = {'a' => '1&b=2', 'c=3&d' => '4'}
+      to_sign = "a=1%26b=2&c%3D3%26d=4"
+      params['hmac'] = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ShopifyAPI::Session.secret, to_sign)
+
+      assert_equal true, ShopifyAPI::Session.validate_signature(params)
+    end
+
+    test "return true when validating signature of params with percent sign characters" do
+      ShopifyAPI::Session.secret = 'secret'
+      params = {'a%3D1%26b' => '2%26c%3D3'}
+      to_sign = "a%253D1%2526b=2%2526c%253D3"
+      params['hmac'] = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), ShopifyAPI::Session.secret, to_sign)
+
+      assert_equal true, ShopifyAPI::Session.validate_signature(params)
+    end
+
     private
 
     def make_sorted_params(params)