#84: Fix Signature Verification Bug (#85)

* fix sig verification bug, clean up code a bit * 1.0.4
mitchwadair · Dec 16, 2023 · a1c8f46 · a1c8f46
1 parent 9785889
commit a1c8f46
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 52 deletions.
diff --git a/lib/whserver.js b/lib/whserver.js
@@ -1,4 +1,4 @@
-// Copyright (c) 2020-2022 Mitchell Adair
+// Copyright (c) 2020-2023 Mitchell Adair
 //
 // This software is released under the MIT License.
 // https://opensource.org/licenses/MIT
@@ -8,63 +8,49 @@ const crypto = require("crypto");
 const EventManager = require("./events");
 const logger = require("./logger");
 
-// correct req.body to be a JSON object if the parent app is using a different parsing middleware at the app level
-const correctBody = (req, _res, next) => {
-    if (Buffer.isBuffer(req.body)) {
-        logger.debug("Convert body from raw");
-        req.body = JSON.parse(Buffer.toString(req.body)); //if app is using express.raw(), convert body to JSON
-    } else if (typeof req.body === "string") {
-        logger.debug("Convert body from string");
-        req.body = JSON.parse(decodeURIComponent(req.body)); //if app is using express.urlencoded() or express.text(), convert body to JSON
-    }
-    next();
-};
-
-const verify = (secret) => {
-    return (req, res, next) => {
-        logger.debug("Verifying webhook request");
-        if (req.headers && req.headers.hasOwnProperty("twitch-eventsub-message-signature")) {
-            logger.debug("Request contains message signature, calculating verification signature");
+const verify = (secret) => (req, res, buf) => {
+    logger.debug("Verifying webhook request");
+    req.valid_signature = false;
+    if (req.headers && req.headers["twitch-eventsub-message-signature"]) {
+        logger.debug("Request contains message signature, calculating verification signature");
 
-            const id = req.headers["twitch-eventsub-message-id"];
-            const timestamp = req.headers["twitch-eventsub-message-timestamp"];
-            const [algo, signature] = req.headers["twitch-eventsub-message-signature"].split("=");
+        const id = req.headers["twitch-eventsub-message-id"];
+        const timestamp = req.headers["twitch-eventsub-message-timestamp"];
+        const [algo, signature] = req.headers["twitch-eventsub-message-signature"].split("=");
 
-            const buf = Buffer.from(JSON.stringify(req.body));
-            const calculatedSignature = crypto
-                .createHmac(algo, secret)
-                .update(id + timestamp + buf)
-                .digest("hex");
+        const calculatedSignature = crypto.createHmac(algo, secret).update(`${id}${timestamp}${buf}`).digest("hex");
 
-            if (calculatedSignature === signature) {
-                logger.debug("Request message signature match");
-                next();
-            } else {
-                logger.debug(
-                    `Request message signature ${signature} does not match calculated signature ${calculatedSignature}`
-                );
-                res.status(403).send("Request signature mismatch");
-            }
+        if (crypto.timingSafeEqual(Buffer.from(calculatedSignature), Buffer.from(signature))) {
+            logger.debug("Request message signature match");
+            req.valid_signature = true;
         } else {
-            logger.debug("Received unauthorized request to webhooks endpoint");
-            res.status(401).send("Unauthorized request to EventSub webhook");
+            logger.debug(
+                `Request message signature ${signature} does not match calculated signature ${calculatedSignature}`
+            );
+            res.status(403).send("Request signature mismatch");
         }
-    };
+    } else {
+        logger.debug("Received unauthorized request to webhooks endpoint");
+        res.status(401).send("Unauthorized request to EventSub webhook");
+    }
 };
 
 module.exports = function (server, secret, config) {
     const whserver = server || express();
     let recentMessageIds = {};
 
-    whserver.post("/teswh/event", express.json(), correctBody, verify(secret), (req, res) => {
+    whserver.post("/teswh/event", express.json({ verify: verify(secret) }), (req, res) => {
+        if (!req.valid_signature) {
+            return;
+        }
+
+        const { challenge, subscription, event } = req.body;
         // handle a webhook verification request
         const messageType = req.headers["twitch-eventsub-message-type"];
-        if (req.body.hasOwnProperty("challenge") && messageType === "webhook_callback_verification") {
-            logger.log(
-                `Received challenge for ${req.body.subscription.type}, ${req.body.subscription.id}. Returning challenge.`
-            );
-            res.status(200).type("text/plain").send(encodeURIComponent(req.body.challenge)); //ensure plain string response
-            EventManager.resolveSubscription(req.body.subscription.id);
+        if (challenge && messageType === "webhook_callback_verification") {
+            logger.log(`Received challenge for ${subscription.type}, ${subscription.id}. Returning challenge.`);
+            res.status(200).type("text/plain").send(encodeURIComponent(challenge)); //ensure plain string response
+            EventManager.resolveSubscription(subscription.id);
             return;
         }
 
@@ -86,20 +72,20 @@ module.exports = function (server, secret, config) {
         // handle different message types
         switch (messageType) {
             case "notification":
-                logger.log(`Received notification for type ${req.body.subscription.type}`);
+                logger.log(`Received notification for type ${subscription.type}`);
                 recentMessageIds[messageId] = true;
                 setTimeout(() => {
                     delete recentMessageIds[messageId];
                 }, 601000);
-                EventManager.fire(req.body.subscription, req.body.event);
+                EventManager.fire(subscription, event);
                 break;
             case "revocation":
-                logger.log(`Received revocation notification for subscription id ${req.body.subscription.id}`);
+                logger.log(`Received revocation notification for subscription id ${subscription.id}`);
                 recentMessageIds[messageId] = true;
                 setTimeout(() => {
                     delete recentMessageIds[messageId];
                 }, 601000);
-                EventManager.fire({ ...req.body.subscription, type: "revocation" }, req.body.subscription);
+                EventManager.fire({ ...subscription, type: "revocation" }, subscription);
                 break;
             default:
                 logger.log(`Received request with unhandled message type ${messageType}`);

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "tesjs",
-  "version": "1.0.3",
+  "version": "1.0.4",
   "description": "A module to streamline the use of Twitch EventSub in Node.js and Web applications",
   "repository": {
     "type": "git",

diff --git a/test/whserver.js b/test/whserver.js
@@ -109,4 +109,11 @@ describe("whserver", () => {
         );
         sinon.assert.notCalled(cb);
     });
+
+    it("responsds with 200 OK when recieving a notification with an & in the payload", async () => {
+        const cb = sinon.spy();
+        tes.on("channel.channel_points_custom_reward_redemption.add", cb);
+        await cmd(`twitch event trigger add-redemption -F ${REDIRECT_URL} -s ${whSecret} -n "test&name"`);
+        sinon.assert.called(cb);
+    });
 });