WIP - demonstrate hacker one problem

Gemfile.lock

@@ -145,6 +145,7 @@ GEM
     marcel (1.1.0)
     method_source (1.0.0)
     mini_mime (1.1.5)
+    mini_portile2 (2.8.9)
     minitest (5.26.2)
     mocha (2.8.0)
       ruby2_keywords (>= 0.0.5)
@@ -161,11 +162,8 @@ GEM
     net-smtp (0.5.1)
       net-protocol
     nio4r (2.7.5)
-    nokogiri (1.18.10-arm64-darwin)
-      racc (~> 1.4)
-    nokogiri (1.18.10-x86_64-darwin)
-      racc (~> 1.4)
-    nokogiri (1.18.10-x86_64-linux-gnu)
+    nokogiri (1.18.10)
+      mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     oj (3.16.13)
       bigdecimal (>= 3.0)
@@ -291,9 +289,8 @@ GEM
       actionpack (>= 6.1)
       activesupport (>= 6.1)
       sprockets (>= 3.0.0)
-    sqlite3 (2.7.4-arm64-darwin)
-    sqlite3 (2.7.4-x86_64-darwin)
-    sqlite3 (2.7.4-x86_64-linux-gnu)
+    sqlite3 (2.7.4)
+      mini_portile2 (~> 2.8.0)
     stringio (3.1.8)
     syntax_tree (6.1.1)
       prettier_print (>= 1.2.0)
@@ -317,6 +314,7 @@ PLATFORMS
   arm64-darwin-21
   arm64-darwin-22
   arm64-darwin-23
+  arm64-darwin-24
   x86_64-darwin-19
   x86_64-darwin-20
   x86_64-darwin-21

lib/shopify_app/controller_concerns/app_proxy_verification.rb

@@ -20,14 +20,21 @@ def query_string_valid?(query_string)
       signature = query_hash.delete("signature")
       return false if signature.nil?
 
+      # Reject requests with array parameters to prevent HMAC signature bypass.
+      # Shopify's App Proxy never sends duplicate query parameters, so any array
+      # values indicate a potential canonicalization attack where an attacker
+      # could swap "ids=1,2" (string) with "ids=1&ids=2" (array) while maintaining
+      # the same signature.
+      return false if query_hash.values.any? { |v| v.is_a?(Array) }
+
       ActiveSupport::SecurityUtils.secure_compare(
         calculated_signature(query_hash),
         signature,
       )
     end
 
     def calculated_signature(query_hash_without_signature)
-      sorted_params = query_hash_without_signature.collect { |k, v| "#{k}=#{Array(v).join(",")}" }.sort.join
+      sorted_params = query_hash_without_signature.collect { |k, v| "#{k}=#{v}" }.sort.join
 
       OpenSSL::HMAC.hexdigest(
         OpenSSL::Digest.new("sha256"),

test/shopify_app/controller_concerns/app_proxy_verification_test.rb

@@ -71,6 +71,48 @@ class AppProxyVerificationTest < ActionController::TestCase
     end
   end
 
+  test "HMAC signature bypass via canonicalization collision is prevented" do
+    with_test_routes do
+      # VULNERABILITY: The calculated_signature method uses Array(v).join(",")
+      # which creates a collision where "1,2" (string) and ["1","2"] (array)
+      # produce the same canonical form, allowing signature replay attacks.
+
+      timestamp = "1466106083"
+      secret = ShopifyApp.configuration.secret
+
+      # Step 1: Attacker captures/knows a valid signature for STRING parameter "ids=1,2"
+      params_for_signing = {
+        "ids" => "1,2",
+        "path_prefix" => "/apps/my-app",
+        "shop" => "some-random-store.myshopify.com",
+        "timestamp" => timestamp,
+      }
+
+      # This replicates the vulnerable canonicalization: Array("1,2").join(",") => "1,2"
+      sorted_params = params_for_signing.collect { |k, v| "#{k}=#{Array(v).join(",")}" }.sort.join
+      valid_signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, sorted_params)
+
+      # Step 2: Verify the original string-based request works
+      string_query = "ids=1,2&path_prefix=/apps/my-app&shop=some-random-store.myshopify.com&" \
+        "timestamp=#{timestamp}&signature=#{valid_signature}"
+      @request.env["QUERY_STRING"] = string_query
+      get :basic
+      assert_response :ok, "Legitimate string parameter request should succeed"
+
+      # Step 3: ATTACK - Send ARRAY ["1", "2"] using the STRING's signature
+      # Rack parses duplicate params (ids=1&ids=2) as an array ["1", "2"]
+      # The vulnerable code: Array(["1","2"]).join(",") => "1,2" (same as string!)
+      array_query = "ids=1&ids=2&path_prefix=/apps/my-app&shop=some-random-store.myshopify.com&" \
+        "timestamp=#{timestamp}&signature=#{valid_signature}"
+      @request.env["QUERY_STRING"] = array_query
+      get :basic
+
+      # Step 4: This MUST return 403 Forbidden to prevent the attack
+      # If this returns 200 OK, the vulnerability exists!
+      assert_response :forbidden, "Array parameter smuggling attack should be rejected"
+    end
+  end
+
   private
 
   def query_string_valid?(query_string)