From 0ab2504fb340df50cc9398ef64b448580b659b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorens=20Le=C3=B3n?= Date: Thu, 9 Jan 2025 01:22:52 +0100 Subject: [PATCH] fix(webhooks): validate Twilio signatures with escaped and unescaped query string values fixes #1059 --- spec/unit/webhooks/webhooks.spec.js | 105 ++++++++++++++++++++++++++++ src/webhooks/webhooks.ts | 65 ++++++++++++----- 2 files changed, 154 insertions(+), 16 deletions(-) create mode 100644 spec/unit/webhooks/webhooks.spec.js diff --git a/spec/unit/webhooks/webhooks.spec.js b/spec/unit/webhooks/webhooks.spec.js new file mode 100644 index 000000000..bb910c123 --- /dev/null +++ b/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); + }); + }); + }); +}); diff --git a/src/webhooks/webhooks.ts b/src/webhooks/webhooks.ts index 2de9e1fae..8b184cb8b 100644 --- a/src/webhooks/webhooks.ts +++ b/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; @@ -102,6 +103,18 @@ function removePort(parsedUrl: URL): string { 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 +192,53 @@ 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( - authToken, - urlWithPort, - params + return ( + validateSignatureWithUrl( + authToken, + twilioHeader, + removePort(urlObject), + params + ) || + validateSignatureWithUrl( + authToken, + twilioHeader, + addPort(urlObject), + params + ) || + validateSignatureWithUrl( + authToken, + twilioHeader, + withLegacyQuerystring(removePort(urlObject)), + params + ) || + validateSignatureWithUrl( + authToken, + twilioHeader, + withLegacyQuerystring(addPort(urlObject)), + params + ) ); +} + +function validateSignatureWithUrl( + authToken: string, + twilioHeader: string, + url: string, + params: Record +) { const signatureWithoutPort = getExpectedTwilioSignature( authToken, - urlWithoutPort, + url, params ); - const validSignatureWithPort = scmp( - Buffer.from(twilioHeader), - Buffer.from(signatureWithPort) - ); - const validSignatureWithoutPort = scmp( - Buffer.from(twilioHeader), - Buffer.from(signatureWithoutPort) - ); - return validSignatureWithoutPort || validSignatureWithPort; + return scmp(Buffer.from(twilioHeader), Buffer.from(signatureWithoutPort)); } export function validateBody(