fix: Validate signatures in Rack middleware for non-form-data payloads (#590)

Co-authored-by: Elise Shanholtz <[email protected]>

Author: gabrielg

lib/rack/twilio_webhook_authentication.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'rack/media_type'
+
 module Rack
   # Middleware that authenticates webhooks from Twilio using the request
   # validator.
@@ -19,6 +21,10 @@ module Rack
   # doesn't validate then the middleware responds immediately with a 403 status.
 
   class TwilioWebhookAuthentication
+    # Rack's FORM_DATA_MEDIA_TYPES can be modified to taste, so we're slightly
+    # more conservative in what we consider form data.
+    FORM_URLENCODED_MEDIA_TYPE = Rack::MediaType.type('application/x-www-form-urlencoded')
+
     def initialize(app, auth_token, *paths, &auth_token_lookup)
       @app = app
       @auth_token = auth_token
@@ -30,7 +36,7 @@ def call(env)
       return @app.call(env) unless env['PATH_INFO'].match(@path_regex)
       request = Rack::Request.new(env)
       original_url = request.url
-      params = request.post? ? request.POST : {}
+      params = extract_params!(request)
       auth_token = @auth_token || get_auth_token(params['AccountSid'])
       validator = Twilio::Security::RequestValidator.new(auth_token)
       signature = env['HTTP_X_TWILIO_SIGNATURE'] || ''
@@ -44,5 +50,23 @@ def call(env)
         ]
       end
     end
+
+    # Extract the params from the the request that we can use to determine the
+    # signature. This _may_ modify the passed in request since it may read/rewind
+    # the body.
+    def extract_params!(request)
+      return {} unless request.post?
+
+      if request.media_type == FORM_URLENCODED_MEDIA_TYPE
+        request.POST
+      else
+        request.body.rewind
+        body = request.body.read
+        request.body.rewind
+        body
+      end
+    end
+
+    private :extract_params!
   end
 end

spec/rack/twilio_webhook_authentication_spec.rb

@@ -31,6 +31,7 @@
       auth_token = 'qwerty'
       account_sid = 12_345
       expect_any_instance_of(Rack::Request).to receive(:post?).and_return(true)
+      expect_any_instance_of(Rack::Request).to receive(:media_type).and_return(Rack::MediaType.type('application/x-www-form-urlencoded'))
       expect_any_instance_of(Rack::Request).to receive(:POST).and_return({ 'AccountSid' => account_sid })
       @middleware = Rack::TwilioWebhookAuthentication.new(@app, nil, /\/voice/) { |asid| auth_token }
       request_validator = double('RequestValidator')
@@ -103,4 +104,92 @@
       expect(status).to be(403)
     end
   end
+
+  describe 'validating non-form-data POST payloads' do
+    it 'should fail if the body does not validate' do
+      middleware = Rack::TwilioWebhookAuthentication.new(@app, 'qwerty', /\/test/)
+      input = StringIO.new('{"message": "a post body that does not match the bodySHA256"}')
+
+      request = Rack::MockRequest.env_for(
+        'https://example.com/test?bodySHA256=79bfb0acaf0045fd30f13d48d4fe296b393d85a3bfbee881a0172b2bd574b11e',
+        method: 'POST',
+        input: input
+      )
+      request['HTTP_X_TWILIO_SIGNATURE'] = '+LYlbGr/VmN84YPJQCuWs+9UA7E='
+      request['CONTENT_TYPE'] = 'application/json'
+
+      status, headers, body = middleware.call(request)
+
+      expect(status).not_to be(200)
+    end
+
+    it 'should validate if the body signature is correct' do
+      middleware = Rack::TwilioWebhookAuthentication.new(@app, 'qwerty', /\/test/)
+      input = StringIO.new('{"message": "a post body"}')
+
+      request = Rack::MockRequest.env_for(
+        'https://example.com/test?bodySHA256=8d90d640c6ba47d595ac56203d7f5c6b511be80fdf44a2055acca75a119b9fd2',
+        method: 'POST',
+        input: input
+      )
+      request['HTTP_X_TWILIO_SIGNATURE'] = 'zR5Oq4f6cijN5oz5bisiVuxYnTU='
+      request['CONTENT_TYPE'] = 'application/json'
+
+      status, headers, body = middleware.call(request)
+
+      expect(status).to be(200)
+    end
+
+    it 'should validate even if a previous middleware read the body first' do
+      middleware = Rack::TwilioWebhookAuthentication.new(@app, 'qwerty', /\/test/)
+      input = StringIO.new('{"message": "a post body"}')
+
+      request = Rack::MockRequest.env_for(
+        'https://example.com/test?bodySHA256=8d90d640c6ba47d595ac56203d7f5c6b511be80fdf44a2055acca75a119b9fd2',
+        method: 'POST',
+        input: input
+      )
+      request['HTTP_X_TWILIO_SIGNATURE'] = 'zR5Oq4f6cijN5oz5bisiVuxYnTU='
+      request['CONTENT_TYPE'] = 'application/json'
+      request['rack.input'].read
+
+      status, headers, body = middleware.call(request)
+
+      expect(status).to be(200)
+    end
+  end
+
+  describe 'validating application/x-www-form-urlencoded POST payloads' do
+    it 'should fail if the body does not validate' do
+      middleware = Rack::TwilioWebhookAuthentication.new(@app, 'qwerty', /\/test/)
+
+      request = Rack::MockRequest.env_for(
+        'https://example.com/test',
+        method: 'POST',
+        params: { 'foo' => 'bar' }
+      )
+      request['HTTP_X_TWILIO_SIGNATURE'] = 'foobarbaz'
+      expect(request['CONTENT_TYPE']).to eq('application/x-www-form-urlencoded')
+
+      status, headers, body = middleware.call(request)
+
+      expect(status).not_to be(200)
+    end
+
+    it 'should validate if the body signature is correct' do
+      middleware = Rack::TwilioWebhookAuthentication.new(@app, 'qwerty', /\/test/)
+
+      request = Rack::MockRequest.env_for(
+        'https://example.com/test',
+        method: 'POST',
+        params: { 'foo' => 'bar' }
+      )
+      request['HTTP_X_TWILIO_SIGNATURE'] = 'TR9Skm9jiF4WVRJznU5glK5I83k='
+      expect(request['CONTENT_TYPE']).to eq('application/x-www-form-urlencoded')
+
+      status, headers, body = middleware.call(request)
+
+      expect(status).to be(200)
+    end
+  end
 end