CVE-2023-43656 (TypeScript, published 2023-09-27T21:15Z)
Description: value: matrix-hookshot is a Matrix bot for connecting to external services like GitHub, GitLab, JIRA, and more. Instances that have enabled transformation functions (those that have `generic.allowJsTransformationFunctions` in their config), may be vulnerable to an attack where it is possible to break out of the `vm2` sandbox and as a result Hookshot will be vulnerable to this. This problem is only likely to affect users who have allowed untrusted users to apply their own transformation functions. If you have only enabled a limited set of trusted users, this threat is reduced (though not eliminated). Version 4.5.0 and above of hookshot include a new sandbox library which should better protect users. Users are advised to upgrade. Users unable to upgrade should disable `generic.allowJsTransformationFunctions` in the config.
Severity: nan (CVSS v3 base score: 9)
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/matrix-org/matrix-hookshot
Files changed:
changelog.d/817.misc
package.json
src/App/BridgeApp.ts
src/Connections/GenericHook.ts
tests/connections/GenericHookTest.ts
web/components/roomConfig/GenericWebhookConfig.tsx
yarn.lock
Commit message: Replace vm2 with quickjs (#817)

* quickjs test

* Replace vm2 with quickjs

* initalise -> initialise

* Remove unused transformation timeout time

* Don't assume quickModule is set

Also use whether it's set as the indicator of whether transformation
functions are allowed, instead of checking the config

* Refactor GenericHookConnectionState validation

- Do it in the constructor instead of in callers
- Make hookId mandatory so as to not drop it on some state updates
- Conflate a state event's state key with a connection state's name,
  which was already the case in practice

* Refactor validateState

* Drop explicit any

Better to infer the type instead

* Always validate transformation fn

* Fix test

* Add changelog

* Fix disposal, validation, and printing

* Fix transformation error string formatting

Also refactor similar code

* Let invalid transformations run & fail

instead of pretending that one was never set

* Restore transformation timeout time

* Don't execute transformation fn when validating it

Instead, only compile it

* Revert unrelated changes

---------

Co-authored-by: Andrew Ferrazzutti <andrewf@element.io>

Vulnerable code:
    "vm2": "^3.9.18",
import { VMScript as Script, NodeVM } from "vm2";
    static validateState(state: Record<string, unknown>, allowJsTransformationFunctions?: boolean): GenericHookConnectionState {
        let transformationFunctionResult: string|undefined;
        if (transformationFunction) {
            if (allowJsTransformationFunctions !== undefined && !allowJsTransformationFunctions) {
                throw new ApiError('Transformation functions are not allowed', ErrCode.DisabledFeature);
            }
            if (typeof transformationFunction !== "string") {
                throw new ApiError('Transformation functions must be a string', ErrCode.BadValue);
            }
            transformationFunctionResult = transformationFunction;
        }
            ...(transformationFunctionResult && {transformationFunction: transformationFunctionResult}),
        const validState = GenericHookConnection.validateState(data, config.generic.allowJsTransformationFunctions || false);
    private transformationFunction?: Script;
        if (state.transformationFunction && config.allowJsTransformationFunctions) {
            this.transformationFunction = new Script(state.transformationFunction);
        const validatedConfig = GenericHookConnection.validateState(stateEv.content as Record<string, unknown>, this.config.allowJsTransformationFunctions || false);
            try {
                this.transformationFunction = new Script(validatedConfig.transformationFunction);
            } catch (ex) {
                await this.messageClient.sendMatrixText(this.roomId, 'Could not compile transformation function:' + ex, "m.text", this.intent.userId);
            msg.plain = "Received webhook data:\n\n" + "```json\n\n" + JSON.stringify(data, null, 2) + "\n\n```";
            msg.html = `<p>Received webhook data:</p><p><pre><code class=\\"language-json\\">${JSON.stringify(data, null, 2)}</code></pre></p>`
        const vm = new NodeVM({
            console: 'off',
            wrapper: 'none',
            wasm: false,
            eval: false,
            timeout: TRANSFORMATION_TIMEOUT_MS,
        });
        vm.setGlobal('HookshotApiVersion', 'v2');
        vm.setGlobal('data', data);
        vm.run(this.transformationFunction);
        const result = vm.getGlobal('result');
        const validatedConfig = GenericHookConnection.validateState(config, this.config.allowJsTransformationFunctions || false);
          plain: \`*Everything is fine*, the counter is under by\${data.maxValue - data.counter}\`,
acorn-walk@^8.1.1, acorn-walk@^8.2.0:
acorn@^8.4.1, acorn@^8.7.0:
vm2@^3.9.18:
  version "3.9.18"
  resolved "https://registry.yarnpkg.com/vm2/-/vm2-3.9.18.tgz#d919848bee191a0410c5cc1c5aac58adfd03ce9a"
  integrity sha512-iM7PchOElv6Uv6Q+0Hq7dcgDtWWT6SizYqVcvol+1WQc+E9HlgTCnPozbQNSP3yDV9oXHQOEQu530w2q/BCVZg==
  dependencies:
    acorn "^8.7.0"
    acorn-walk "^8.2.0"


Fixed code:
Use quickjs instead of vm2 for evaluating JS transformation functions.
    "quickjs-emscripten": "^0.23.0",
import { GenericHookConnection } from "../Connections";
    if (config.generic?.allowJsTransformationFunctions) {
        await GenericHookConnection.initialiseQuickJS();
    }

import { QuickJSRuntime, QuickJSWASMModule, newQuickJSWASMModule, shouldInterruptAfterDeadline } from "quickjs-emscripten";
    private static quickModule?: QuickJSWASMModule;

    public static async initialiseQuickJS() {
        GenericHookConnection.quickModule = await newQuickJSWASMModule();
    }
    static validateState(state: Record<string, unknown>): GenericHookConnectionState {
        // Use !=, not !==, to check for both undefined and null
        if (transformationFunction != undefined) {
            if (!this.quickModule) {
                throw new ApiError('Transformation functions are not allowed', ErrCode.DisabledFeature);
            }
            if (typeof transformationFunction !== "string") {
                throw new ApiError('Transformation functions must be a string', ErrCode.BadValue);
            }
        }
            ...(transformationFunction && {transformationFunction}),
        const validState = GenericHookConnection.validateState(data);
    private transformationFunction?: string;
    /**
     * @param state Should be a pre-validated state object returned by {@link validateState}
     */
        if (state.transformationFunction && GenericHookConnection.quickModule) {
            this.transformationFunction = state.transformationFunction;
        const validatedConfig = GenericHookConnection.validateState(stateEv.content as Record<string, unknown>);
            const ctx = GenericHookConnection.quickModule!.newContext();
            const codeEvalResult = ctx.evalCode(`function f(data) {${validatedConfig.transformationFunction}}`, undefined, { compileOnly: true });
            if (codeEvalResult.error) {
                const errorString = JSON.stringify(ctx.dump(codeEvalResult.error), null, 2);
                codeEvalResult.error.dispose();
                ctx.dispose();

                const errorPrefix = "Could not compile transformation function:";
                await this.intent.sendEvent(this.roomId, {
                    msgtype: "m.text",
                    body: errorPrefix + "\n\n```json\n\n" + errorString + "\n\n```",
                    formatted_body: `<p>${errorPrefix}</p><p><pre><code class=\\"language-json\\">${errorString}</code></pre></p>`,
                    format: "org.matrix.custom.html",
                });
            } else {
                codeEvalResult.value.dispose();
                ctx.dispose();
                this.transformationFunction = validatedConfig.transformationFunction;
            const dataString = JSON.stringify(data, null, 2);
            const dataPrefix = "Received webhook data:";
            msg.plain = dataPrefix + "\n\n```json\n\n" + dataString + "\n\n```";
            msg.html = `<p>${dataPrefix}</p><p><pre><code class=\\"language-json\\">${dataString}</code></pre></p>`
        let result;
        const ctx = GenericHookConnection.quickModule!.newContext();
        ctx.runtime.setInterruptHandler(shouldInterruptAfterDeadline(Date.now() + TRANSFORMATION_TIMEOUT_MS));
        try {
            ctx.setProp(ctx.global, 'HookshotApiVersion', ctx.newString('v2'));
            const ctxResult = ctx.evalCode(`const data = ${JSON.stringify(data)};\n\n${this.state.transformationFunction}`);

            if (ctxResult.error) {
                const e = Error(`Transformation failed to run: ${JSON.stringify(ctx.dump(ctxResult.error))}`);
                ctxResult.error.dispose();
                throw e;
            } else {
                const value = ctx.getProp(ctx.global, 'result');
                result = ctx.dump(value);
                value.dispose();
                ctxResult.value.dispose();
            }
        } finally {
            ctx.global.dispose();
            ctx.dispose();
        }
        const validatedConfig = GenericHookConnection.validateState(config);
    before(async () => {
        await GenericHookConnection.initialiseQuickJS();
    })
          plain: \`*Everything is fine*, the counter is under by \${data.maxValue - data.counter}\`,
acorn-walk@^8.1.1:
acorn@^8.4.1:
quickjs-emscripten@^0.23.0:
  version "0.23.0"
  resolved "https://registry.yarnpkg.com/quickjs-emscripten/-/quickjs-emscripten-0.23.0.tgz#94f412d0ee5f3021fc12ddf6c0b00bd8ce0d28b9"
  integrity sha512-CIP+NDRYDDqbT3cTiN8Bon1wsZ7IgISVYCJHYsPc86oxszpepVMPXFfttyQgn1u1okg1HPnCnM7Xv1LrCO/VmQ==