Protect Draupnir from matrix-bot-sdk

src/utils.ts

@@ -236,7 +236,7 @@ function patchMatrixClientForConciseExceptions() {
         // We'll only throw the error if there is one.
         let error = new Error("STACK CAPTURE");
         originalRequestFn(params, function conciseExceptionRequestFn(
-            err: { [key: string]: any }, response: { [key: string]: any }, resBody: string
+            err: { [key: string]: unknown }, response: { [key: string]: any }, resBody: unknown
         ) {
             if (!err && (response?.statusCode < 200 || response?.statusCode >= 300)) {
                 // Normally, converting HTTP Errors into rejections is done by the caller
@@ -272,34 +272,36 @@ function patchMatrixClientForConciseExceptions() {
             // useless.
             let method: string | null = null;
             let path = '';
-            let body: string | null = null;
+            let body: unknown = null;
             if (err.method) {
                 method = err.method;
             }
             if (err.url) {
                 path = err.url;
             }
-            if ("req" in err && (err as any).req instanceof ClientRequest) {
+            if ("req" in err && err.req instanceof ClientRequest) {
                 if (!method) {
-                    method = (err as any).req.method;
+                    method = err.req.method;
                 }
                 if (!path) {
-                    path = (err as any).req.path;
+                    path = err.req.path;
                 }
             }
             if ("body" in err) {
-                body = (err as any).body;
+                body = err.body;
             }
-            let message = `Error during MatrixClient request ${method} ${path}: ${err.statusCode} ${err.statusMessage} -- ${body}`;
-            error.message = message;
-            if (body) {
-                // Calling code may use `body` to check for errors, so let's
-                // make sure that we're providing it.
+            // Calling code may use `body` to check for errors, so let's
+            // make sure that we're providing it.
+            if (typeof body === 'string') {
                 try {
-                    body = JSON.parse(body);
+                    body = JSON.parse(body, jsonReviver);
                 } catch (ex) {
                     // Not JSON.
                 }
+            }
+            let message = `Error during MatrixClient request ${method} ${path}: ${err.statusCode} ${err.statusMessage} -- ${JSON.stringify(body)}`;
+            error.message = message;
+            if (body) {
                 // Define the property but don't make it visible during logging.
                 Object.defineProperty(error, "body", {
                     value: body,
@@ -355,7 +357,7 @@ function patchMatrixClientForRetry() {
                 try {
                     let result: any[] = await new Promise((resolve, reject) => {
                         originalRequestFn(params, function requestFnWithRetry(
-                            err: { [key: string]: any }, response: { [key: string]: any }, resBody: string
+                            err: { [key: string]: any }, response: { [key: string]: unknown }, resBody: unknown
                         ) {
                             // Note: There is no data race on `attempt` as we `await` before continuing
                             // to the next iteration of the loop.
@@ -398,20 +400,72 @@ function patchMatrixClientForRetry() {
     isMatrixClientPatchedForRetryWhenThrottled = true;
 }
 
+let isMatrixClientPatchedForPrototypePollution = false;
+
+function jsonReviver(key: string, value: any): any {
+    if (key === '__proto__' || key === 'constructor') {
+        return undefined;
+    } else {
+        return value;
+    }
+}
+
+/**
+ * https://github.com/turt2live/matrix-bot-sdk/blob/c7d16776502c26bbb547a3d667ec92eb50e7026c/src/http.ts#L77-L101 💀 fucking hell!!!!
+ *
+ * The following is an inefficient workaround, but you gotta do what you can.
+ */
+function patchMatrixClientForPrototypePollution() {
+    if (isMatrixClientPatchedForPrototypePollution) {
+        return;
+    }
+    const originalRequestFn = getRequestFn();
+    setRequestFn((params: { [k: string]: any }, cb: any) => {
+        originalRequestFn(params, function conciseExceptionRequestFn(
+            error: { [key: string]: any }, response: { [key: string]: any }, resBody: unknown
+        ) {
+            // https://github.com/turt2live/matrix-bot-sdk/blob/c7d16776502c26bbb547a3d667ec92eb50e7026c/src/http.ts#L77-L101
+            // bring forwards this step and do it safely.
+            if (typeof resBody === 'string') {
+                try {
+                    resBody = JSON.parse(resBody, jsonReviver);
+                } catch (e) {
+                    // we don't care if we fail to parse the JSON as it probably isn't JSON.
+                }
+            }
+
+            if (typeof response.body === 'string') {
+                try {
+                    response.body = JSON.parse(response.body, jsonReviver);
+                } catch (e) {
+                    // we don't care if we fail to parse the JSON as it probably isn't JSON.
+                }
+            }
+            return cb(error, response, resBody);
+        })
+    });
+    isMatrixClientPatchedForPrototypePollution = true;
+}
+
 /**
  * Perform any patching deemed necessary to MatrixClient.
  */
 export function patchMatrixClient() {
     // Note that the order of patches is meaningful.
     //
+    // - `patchMatrixClientForPrototypePollution` converts all JSON bodies to safe JSON before client code can
+    //    parse and use the JSON inappropriately.
     // - `patchMatrixClientForConciseExceptions` converts all `IncomingMessage`
     //   errors into instances of `Error` handled as errors;
     // - `patchMatrixClientForRetry` expects that all errors are returned as
     //   errors.
+    patchMatrixClientForPrototypePollution();
     patchMatrixClientForConciseExceptions();
     patchMatrixClientForRetry();
 }
 
+patchMatrixClient();
+
 /**
  * Initialize Sentry for error monitoring and reporting.
  *

test/integration/clientHelper.ts

@@ -1,5 +1,6 @@
 import { HmacSHA1 } from "crypto-js";
 import { getRequestFn, LogService, MatrixClient, MemoryStorageProvider, PantalaimonClient } from "matrix-bot-sdk";
+import "../../src/utils"; // we need this for the patches to matrix-bot-sdk's `getRequestFn`.
 
 const REGISTRATION_ATTEMPTS = 10;
 const REGISTRATION_RETRY_BASE_DELAY_MS = 100;
@@ -17,12 +18,17 @@ const REGISTRATION_RETRY_BASE_DELAY_MS = 100;
  */
 export async function registerUser(homeserver: string, username: string, displayname: string, password: string, admin: boolean): Promise<void> {
     let registerUrl = `${homeserver}/_synapse/admin/v1/register`
-    const data: {nonce: string} = await new Promise((resolve, reject) => {
-        getRequestFn()({uri: registerUrl, method: "GET", timeout: 60000}, (error: any, response: any, resBody: any) => {
-            error ? reject(error) : resolve(JSON.parse(resBody))
+    const nonce: string = await new Promise((resolve, reject) => {
+        getRequestFn()({uri: registerUrl, method: "GET", timeout: 60000}, (error: any, _response: any, resBody: unknown) => {
+            if (error) {
+                reject(error);
+            } else if (typeof resBody === 'object' && resBody !== null && 'nonce' in resBody && typeof resBody.nonce === 'string') {
+                resolve(resBody.nonce)
+            } else {
+                reject(new TypeError(`Don't know what to do with response body ${JSON.stringify(resBody)}`));
+            }
         });
     });
-    const nonce = data.nonce!;
     let mac = HmacSHA1(`${nonce}\0${username}\0${password}\0${admin ? 'admin' : 'notadmin'}`, 'REGISTRATION_SHARED_SECRET');
     for (let i = 1; i <= REGISTRATION_ATTEMPTS; ++i) {
         try {
@@ -136,7 +142,7 @@ async function getGlobalAdminUser(homeserver: string): Promise<MatrixClient> {
         try {
             await registerUser(homeserver, USERNAME, USERNAME, USERNAME, true);
         } catch (e) {
-            if (e.isAxiosError && e?.response?.data?.errcode === 'M_USER_IN_USE') {
+            if (e?.body?.errcode === 'M_USER_IN_USE') {
                 // Then we've already registered the user in a previous run and that is ok.
             } else {
                 throw e;

test/integration/commands/shutdownCommandTest.ts

@@ -47,8 +47,10 @@ describe("Test: shutdown command", function() {
         await reply1
         await reply2
 
-        await assert.rejects(client.joinRoom(badRoom), e => {
-            return e.message.endsWith('{"errcode":"M_UNKNOWN","error":"This room has been blocked on this server"}');
+        await assert.rejects(client.joinRoom(badRoom), (e: any) => {
+            assert.equal(e.statusCode, 403);
+            assert.equal(e.body.error, "This room has been blocked on this server");
+            return true;
         });
     });
 });

test/integration/fixtures.ts

@@ -1,6 +1,9 @@
 import { read as configRead } from "../../src/config";
+import { patchMatrixClient } from "../../src/utils";
 import { makeMjolnir, teardownManagementRoom } from "./mjolnirSetupUtils";
 
+patchMatrixClient();
+
 // When Mjolnir starts (src/index.ts) it clobbers the config by resolving the management room
 // alias specified in the config (config.managementRoom) and overwriting that with the room ID.
 // Unfortunately every piece of code importing that config imports the same instance, including

test/integration/mjolnirSetupUtils.ts

@@ -26,6 +26,8 @@ import { overrideRatelimitForUser, registerUser } from "./clientHelper";
 import { initializeSentry, patchMatrixClient } from "../../src/utils";
 import { IConfig } from "../../src/config";
 
+patchMatrixClient();
+
 /**
  * Ensures that a room exists with the alias, if it does not exist we create it.
  * @param client The MatrixClient to use to resolve or create the aliased room.
@@ -82,7 +84,6 @@ export async function makeMjolnir(config: IConfig): Promise<Mjolnir> {
     const pantalaimon = new PantalaimonClient(config.homeserverUrl, new MemoryStorageProvider());
     const client = await pantalaimon.createClientWithCredentials(config.pantalaimon.username, config.pantalaimon.password);
     await overrideRatelimitForUser(config.homeserverUrl, await client.getUserId());
-    patchMatrixClient();
     await ensureAliasedRoomExists(client, config.managementRoom);
     let mj = await Mjolnir.setupMjolnirFromConfig(client, client, config);
     globalClient = client;

test/integration/policyConsumptionTest.ts

@@ -13,11 +13,11 @@ async function currentRules(mjolnir: Mjolnir): Promise<{ start: object, stop: ob
     return await new Promise((resolve, reject) => getRequestFn()({
             uri: `http://${mjolnir.config.web.address}:${mjolnir.config.web.port}/api/1/ruleserver/updates/`,
             method: "GET"
-    }, (error: object, _response: any, body: string) => {
+    }, (error: object, _response: any, body: any) => {
         if (error) {
             reject(error)
         } else {
-            resolve(JSON.parse(body))
+            resolve(body)
         }
     }));
 }