Closed gabrielg closed 2 years ago
Hi @gabrielg, thanks so much for this fix. I wrote the original rack middleware here and it was an oversight not testing other content types. I just wanted to add my thanks for your noticing and fixing of this code and for documenting it and for the examples in this PR. ❤️❤️❤️
The webhook authentication middleware has been using the return value of
request.POST
to determine the params to validate, assuming it returns a non-Hash
in the case of a non-formish-POST. However, this is not true, though perhaps it was at some point in the past:https://github.com/rack/rack/blob/8c9a97fe6073a8f4c17f85fff20f20eae22d1bcb/lib/rack/request.rb#L451-L478
This means the
RequestValidator
body validation code is entirely skipped over, sinceparams_hash
is alwaysEnumerable
:https://github.com/twilio/twilio-ruby/blob/6c50e0fa13bfefdcdaaa0799b8798023ce492e60/lib/twilio-ruby/security/request_validator.rb#L34-L39
So the body is always considered valid, and only the URL signature is ever validated:
https://github.com/twilio/twilio-ruby/blob/6c50e0fa13bfefdcdaaa0799b8798023ce492e60/lib/twilio-ruby/security/request_validator.rb#L32 https://github.com/twilio/twilio-ruby/blob/6c50e0fa13bfefdcdaaa0799b8798023ce492e60/lib/twilio-ruby/security/request_validator.rb#L46
This fixes the problem by being strict about what's considered a form, and actually reading the body as the params in the case the middleware has not received a form.
I don't actually know how Twilio calculates signatures for
multipart/form-data
requests, if it ever makes them at all, so these aren't treated any differently than any other nonapplication/x-www-form-urlencoded
body.Proof of Concept
gabrielg/twilio-ruby-signature-issue
is a Sinatra app I prepared earlier.main
has the broken code,fixed
vendors this branch.I needed a JSON payload, so I set up an Event Streams sink and subscription and sent a message to a number. Here's what that looks like, with the
from
andto
numbers redacted for privacy:I can take that signature and URL, set the
Content-Type
toapplication/json
, andcurl
anything and have it pass signature validation:A bad actor thus only needs a valid signature and URL to make requests, not the original auth token used for signing.
I also set up another sink and subscription against a Heroku app running the
fixed
branch. An incoming message against it looks like:If I take that signature and URL and attempt to tamper with the body, validation now fails, as expected:
FWIW: I tried to disclose this problem and the fix via Bugcrowd but was told that non-validation of webhook signatures is not, in fact, considered a security issue.
Checklist