fix(webhooks): validate Twilio signatures with escaped and unescaped query string values fixes #1059

spec/unit/webhooks/webhooks.spec.js

@@ -0,0 +1,105 @@
+import { getExpectedTwilioSignature, validateRequest } from "../../../src";
+
+describe("webhooks", () => {
+  const authToken = "s3cr3t";
+
+  describe("validateRequest()", () => {
+    it("should return false when the signature URL does not match the target URL", () => {
+      const serverUrl = "https://example.com/path?test=param";
+      const targetUrl = "https://example.com/path?test=param2";
+
+      const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+      const result = validateRequest(authToken, signature, targetUrl, {});
+
+      expect(result).toBe(false);
+    });
+
+    describe("when the signature is derived from an URL with port", () => {
+      it("should return true when the target url contains the port", () => {
+        const serverUrl = "https://example.com:443/path?test=param";
+        const targetUrl = "https://example.com:443/path?test=param";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+
+      it("should return true when the target url does not contain the port", () => {
+        const serverUrl = "https://example.com:443/path?test=param";
+        const targetUrl = "https://example.com/path?test=param";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+    });
+
+    describe("when the signature is derived from an URL without port", () => {
+      it("should return true when the target url does not contain the port", () => {
+        const serverUrl = "https://example.com/path?test=param";
+        const targetUrl = "https://example.com/path?test=param";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+
+      it("should return true when the target url contains the port", () => {
+        const serverUrl = "https://example.com/path?test=param";
+        const targetUrl = "https://example.com:443/path?test=param";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+    });
+
+    describe("when the signature is derived from an URL with a query param containing an unescaped single quote", () => {
+      it("should return true when the target url contains the unescaped single quote", () => {
+        const serverUrl = "https://example.com/path?test=param'WithQuote";
+        const targetUrl = "https://example.com/path?test=param'WithQuote";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+
+      it("should return true when the target url contains the escaped single quote", () => {
+        const serverUrl = "https://example.com/path?test=param'WithQuote";
+        const targetUrl = "https://example.com/path?test=param%27WithQuote";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+    });
+
+    describe("when the signature is derived from an URL with a query param containing an escaped single quote", () => {
+      it("should return true when the target url contains the unescaped single quote", () => {
+        const serverUrl = "https://example.com/path?test=param%27WithQuote";
+        const targetUrl = "https://example.com/path?test=param'WithQuote";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+
+      it("should return true when the target url contains the escaped single quote", () => {
+        const serverUrl = "https://example.com/path?test=param%27WithQuote";
+        const targetUrl = "https://example.com/path?test=param%27WithQuote";
+
+        const signature = getExpectedTwilioSignature(authToken, serverUrl, {});
+        const result = validateRequest(authToken, signature, targetUrl, {});
+
+        expect(result).toBe(true);
+      });
+    });
+  });
+});

src/webhooks/webhooks.ts

@@ -2,6 +2,7 @@ const scmp = require("scmp");
 import crypto from "crypto";
 import urllib from "url";
 import { IncomingHttpHeaders } from "http2";
+import { parse, stringify } from "querystring";
 
 export interface Request {
   protocol: string;
@@ -98,10 +99,24 @@ function addPort(parsedUrl: URL): string {
  @returns URL without port
  */
 function removePort(parsedUrl: URL): string {
+  parsedUrl = new URL(parsedUrl); // prevent mutation of original URL object
+
   parsedUrl.port = "";
   return parsedUrl.toString();
 }
 
+function withLegacyQuerystring(url: string): string {
+  const parsedUrl = new URL(url);
+
+  if (parsedUrl.search) {
+    const qs = parse(parsedUrl.search.slice(1));
+    parsedUrl.search = "";
+    return parsedUrl.toString() + "?" + stringify(qs);
+  }
+
+  return url;
+}
+
 /**
  Utility function to convert request parameter to a string format
 
@@ -179,33 +194,70 @@ export function validateRequest(
 ): boolean {
   twilioHeader = twilioHeader || "";
   const urlObject = new URL(url);
-  const urlWithPort = addPort(urlObject);
-  const urlWithoutPort = removePort(urlObject);
 
   /*
    *  Check signature of the url with and without the port number
+   *  and with and without the legacy querystring (special chars are encoded when using `new URL()`)
    *  since signature generation on the back end is inconsistent
    */
-  const signatureWithPort = getExpectedTwilioSignature(
+  const isValidSignatureWithoutPort = validateSignatureWithUrl(
     authToken,
-    urlWithPort,
+    twilioHeader,
+    removePort(urlObject),
     params
   );
-  const signatureWithoutPort = getExpectedTwilioSignature(
+
+  if (isValidSignatureWithoutPort) {
+    return true;
+  }
+
+  const isValidSignatureWithPort = validateSignatureWithUrl(
     authToken,
-    urlWithoutPort,
+    twilioHeader,
+    addPort(urlObject),
     params
   );
-  const validSignatureWithPort = scmp(
-    Buffer.from(twilioHeader),
-    Buffer.from(signatureWithPort)
-  );
-  const validSignatureWithoutPort = scmp(
-    Buffer.from(twilioHeader),
-    Buffer.from(signatureWithoutPort)
+
+  if (isValidSignatureWithPort) {
+    return true;
+  }
+
+  const isValidSignatureWithLegacyQuerystringWithoutPort =
+    validateSignatureWithUrl(
+      authToken,
+      twilioHeader,
+      withLegacyQuerystring(removePort(urlObject)),
+      params
+    );
+
+  if (isValidSignatureWithLegacyQuerystringWithoutPort) {
+    return true;
+  }
+
+  const isValidSignatureWithLegacyQuerystringWithPort =
+    validateSignatureWithUrl(
+      authToken,
+      twilioHeader,
+      withLegacyQuerystring(addPort(urlObject)),
+      params
+    );
+
+  return isValidSignatureWithLegacyQuerystringWithPort;
+}
+
+function validateSignatureWithUrl(
+  authToken: string,
+  twilioHeader: string,
+  url: string,
+  params: Record<string, any>
+): boolean {
+  const signatureWithoutPort = getExpectedTwilioSignature(
+    authToken,
+    url,
+    params
   );
 
-  return validSignatureWithoutPort || validSignatureWithPort;
+  return scmp(Buffer.from(twilioHeader), Buffer.from(signatureWithoutPort));
 }
 
 export function validateBody(