Twilio webhook signature validation can return false negative when parameters with arrays are passed

[KyleLehtinenDev]

Issue Summary

While working on a Conversations webhook we noticed that twilio signature validation would occasionally return a false negative with certain combinations of parameters, in our case the "MessagingBinding.Address" array with group messaging.

Digging into this we discovered that the sorting logic doesn't account for the values of like-named parameters, leading to occurrences of signatures not matching between the Twilio header and the expected signature from the Twilio SDK validateSignature helper method.

Below is an example of a constructed string read from breakpointing the method which leads to a 'false' validation result:

`https://xxx.ngrok.io:443/api/v1/chat/conversations\
AccountSidACxxx\
Attributes{}\
ChatServiceSidISxxx\
EventTypeonConversationAdd\
MessageBodyfsg\
MessagingBinding.Address+18550000000\
MessagingBinding.Address+18330000000\
MessagingBinding.AuthorAddress+16020000000\
MessagingServiceSidMGxxx\
RetryCount0\
SourceSMS\
Stateinitializing`

Taking the value returned from toFormUrlEncodedParam, manually arranging the order of each of the "MessagingBinding.Address" parameters, and running the crypto function to compare returned a matching signature sent with the original request.

Altered string that passes

`https://xxx.ngrok.io:443/api/v1/chat/conversations\
AccountSidACxxx\
Attributes{}\
ChatServiceSidISxxx\
EventTypeonConversationAdd\
MessageBodyfsg\
MessagingBinding.Address+18330000000\ <-- Swapped
MessagingBinding.Address+18550000000\ <-- Swapped
MessagingBinding.AuthorAddress+16020000000\
MessagingServiceSidMGxxx\
RetryCount0\
SourceSMS\
Stateinitializing`

Steps to Reproduce

To recreate our scenario:

1. Setup a webhook route on a test server that will receive Twilio Conversation events. We encountered this issue in the "onConversationAdd" pre-event.
2. Have a Twilio number point to a message service that uses conversations and has the "auto create conversation" integration setting enabled.
3. Send a group message with more than two recipients. The more recipient phone numbers added the more likely you will encounter this problem as it depends on how the .sort on the "MessagingBinding.Address" parameter comes out.

We were able to prevent this issue from occurring if we checked for any array parameters and pre-sorted their items before passing it into the validateRequest method like below:

if (params["MessagingBinding.Address"]) {
    params["MessagingBinding.Address"].sort();
 }
 valid = validateRequest(
          TWILIO_AUTH_TOKEN,
          twilioSignature,
          url,
          params
        );

Looking at the toFormUrlEncodedParam method there is a check to see if the parameter is an array before recursively mapping the array values for the accumulated string. Adding a sort before the .map call would ensure array values are pre-sorted to match the signature returned from Twilio server:

function toFormUrlEncodedParam(paramName, paramValue) {
  if (paramValue instanceof Array) {
    return paramValue
      .sort() // <---- Added sort 
      .map(val => toFormUrlEncodedParam(paramName, val))
      .reduce((acc, val) => acc + val, '');
  }
  return paramName + paramValue;
}

Technical details:

• twilio-node version: ^3.67.2
• node version: 16

[shwetha-manvinkurke]

@KyleLehtinenDev thanks for bringing this to our attention. This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

[KyleLehtinenDev]

@shwetha-manvinkurke one of my peers opened a PR for this issue for your consideration.

[hugo-netcraft]

Hi, our team just came across the issue as well, with Autopilot picking up multiple parameters for a field type. Seems to be an issue in all of the Twilio language modules that I have tested (and probably more from the quick skimming of the code) and not mentioned in the security documentation.

Assuming Twilio is using this module to validate the requests in their new feature "Functions", this will be the reason why it is rejecting any requests multiple values set to one parameter.

[hugo-netcraft]

Hi just like to link this message as it seems this issue might not be completely fixed