fix(cactus-common): coerceUnknownToError() now uses HTML sanitize

packages/cactus-common/src/main/typescript/exception/coerce-unknown-to-error.ts

@@ -1,4 +1,5 @@
 import stringify from "fast-safe-stringify";
+import sanitizeHtml from "sanitize-html";
 import { ErrorFromUnknownThrowable } from "./error-from-unknown-throwable";
 import { ErrorFromSymbol } from "./error-from-symbol";
 
@@ -26,10 +27,11 @@ export function coerceUnknownToError(x: unknown): Error {
   } else if (x instanceof Error) {
     return x;
   } else {
-    const xAsJson = stringify(x, (_, value) =>
+    const xAsJsonUnsafe = stringify(x, (_, value) =>
       typeof value === "bigint" ? value.toString() + "n" : value,
     );
-    return new ErrorFromUnknownThrowable(xAsJson);
+    const xAsJsonSanitized = sanitizeHtml(xAsJsonUnsafe);
+    return new ErrorFromUnknownThrowable(xAsJsonSanitized);
   }
 }
 

packages/cactus-core/package.json

@@ -56,13 +56,15 @@
     "express-jwt-authz": "2.4.1",
     "express-openapi-validator": "5.0.4",
     "http-errors": "2.0.0",
+    "http-errors-enhanced": "1.1.2",
     "run-time-error-cjs": "1.4.0",
     "safe-stable-stringify": "2.4.3",
     "typescript-optional": "2.0.1"
   },
   "devDependencies": {
     "@types/express": "4.17.19",
     "@types/http-errors": "2.0.2",
+    "node-mocks-http": "1.14.0",
     "uuid": "8.3.2"
   },
   "engines": {

packages/cactus-core/src/main/typescript/web-services/handle-rest-endpoint-exception.ts

@@ -1,12 +1,13 @@
 import {
   Logger,
   createRuntimeErrorWithCause,
+  safeStringifyException,
 } from "@hyperledger/cactus-common";
 import type { Response } from "express";
 import createHttpError from "http-errors";
 
 /**
- * An interface describing the object contaiing the contextual information needed by the
+ * An interface describing the object containing the contextual information needed by the
  * `#handleRestEndpointException()` method to perform its duties.
  *
  * @param ctx - An object containing options for handling the REST endpoint exception.
@@ -35,39 +36,46 @@ export interface IHandleRestEndpointExceptionOptions {
  *
  * @param ctx - An object containing options for handling the REST endpoint exception.
  */
-export function handleRestEndpointException(
+export async function handleRestEndpointException(
   ctx: Readonly<IHandleRestEndpointExceptionOptions>,
-): void {
+): Promise<void> {
+  const errorAsSanitizedJson = safeStringifyException(ctx.error);
+
+  const { identifierByCodes, INTERNAL_SERVER_ERROR } = await import(
+    "http-errors-enhanced"
+  );
+
   if (createHttpError.isHttpError(ctx.error)) {
     ctx.res.status(ctx.error.statusCode);
 
     // Log either an error or a debug message depending on what the statusCode is
     // For 5xx errors we treat it as a production bug that needs to be fixed on
     // our side and for everything else we treat it a user error and debug log it.
-    if (ctx.error.statusCode >= 500) {
-      ctx.log.debug(ctx.errorMsg, ctx.error);
+    if (ctx.error.statusCode >= INTERNAL_SERVER_ERROR) {
+      ctx.log.debug(ctx.errorMsg, errorAsSanitizedJson);
     } else {
-      ctx.log.error(ctx.errorMsg, ctx.error);
+      ctx.log.error(ctx.errorMsg, errorAsSanitizedJson);
     }
 
     // If the `expose` property is set to true it implies that we can safely
     // expose the contents of the exception to the calling client.
     if (ctx.error.expose) {
       ctx.res.json({
-        message: ctx.error.message,
-        error: ctx.error,
+        message: identifierByCodes[ctx.error.statusCode],
+        error: errorAsSanitizedJson,
       });
     }
   } else {
     // If the exception was not an http-error then we assume it was an internal
     // error (e.g. same behavior as if we had received an HTTP 500 statusCode)
-    ctx.log.error(ctx.errorMsg, ctx.error);
+    ctx.log.error(ctx.errorMsg, errorAsSanitizedJson);
 
     const rex = createRuntimeErrorWithCause(ctx.errorMsg, ctx.error);
+    const sanitizedJsonRex = safeStringifyException(rex);
 
-    ctx.res.status(500).json({
-      message: "Internal Server Error",
-      error: rex,
+    ctx.res.status(INTERNAL_SERVER_ERROR).json({
+      message: identifierByCodes[INTERNAL_SERVER_ERROR],
+      error: sanitizedJsonRex,
     });
   }
 }

packages/cactus-core/src/test/typescript/unit/handle-rest-endpoint-exception.test.ts

@@ -0,0 +1,211 @@
+import "jest-extended";
+import createHttpError from "http-errors";
+import { createResponse } from "node-mocks-http";
+
+import { safeStringifyException } from "@hyperledger/cactus-common";
+
+import {
+  handleRestEndpointException,
+  IHandleRestEndpointExceptionOptions,
+} from "../../../main/typescript/public-api"; // replace with the correct path to your module
+
+import { LoggerProvider } from "@hyperledger/cactus-common";
+
+const log = LoggerProvider.getOrCreate({
+  label: "handle-rest-endpoint-exception.test.ts",
+  level: "DEBUG",
+});
+
+describe("handleRestEndpointException", () => {
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it("should handle HttpError with statusCode >= 500", async () => {
+    const mockResponse = createResponse();
+
+    const mockOptions: IHandleRestEndpointExceptionOptions = {
+      errorMsg: "Test error message",
+      log: jest.mocked(log), // Provide a mock logger if needed
+      error: new Error("Test error"), // Provide appropriate error objects for testing
+      res: mockResponse,
+    };
+
+    const mockHttpError = createHttpError(500, "Test HTTP error", {
+      expose: true,
+    });
+
+    const errorAsSanitizedJson = safeStringifyException(mockHttpError);
+    const spyLogDebug = jest.spyOn(mockOptions.log, "debug");
+    const spyStatus = jest.spyOn(mockResponse, "status");
+    const spyJson = jest.spyOn(mockResponse, "json");
+
+    await handleRestEndpointException({ ...mockOptions, error: mockHttpError });
+
+    expect(spyStatus).toHaveBeenCalledWith(500);
+
+    expect(spyLogDebug).toHaveBeenCalledWith(
+      mockOptions.errorMsg,
+      errorAsSanitizedJson,
+    );
+
+    expect(spyJson).toHaveBeenCalledWith({
+      message: "InternalServerError",
+      error: errorAsSanitizedJson,
+    });
+  });
+
+  it("should handle HttpError with statusCode < 500", async () => {
+    const mockResponse = createResponse();
+
+    const mockOptions: IHandleRestEndpointExceptionOptions = {
+      errorMsg: "Test error message",
+      log: jest.mocked(log), // Provide a mock logger if needed
+      error: new Error("Test error"), // Provide appropriate error objects for testing
+      res: mockResponse,
+    };
+
+    const mockHttpError = createHttpError(404, "Test HTTP error", {
+      expose: true,
+    });
+
+    const errorAsSanitizedJson = safeStringifyException(mockHttpError);
+    const spyLogError = jest.spyOn(mockOptions.log, "error");
+    const spyStatus = jest.spyOn(mockResponse, "status");
+    const spyJson = jest.spyOn(mockResponse, "json");
+    await handleRestEndpointException({ ...mockOptions, error: mockHttpError });
+
+    expect(spyStatus).toHaveBeenCalledWith(404);
+    expect(spyLogError).toHaveBeenCalledWith(
+      mockOptions.errorMsg,
+      errorAsSanitizedJson,
+    );
+
+    expect(spyJson).toHaveBeenCalledWith({
+      message: "NotFound",
+      error: errorAsSanitizedJson,
+    });
+  });
+
+  it("should handle non-HttpError", async () => {
+    const mockResponse = createResponse();
+
+    const mockError = new Error("An unexpected exception. Ha!");
+
+    const mockOptions: IHandleRestEndpointExceptionOptions = {
+      errorMsg: "Test error message",
+      log: jest.mocked(log), // Provide a mock logger if needed
+      error: mockError,
+      res: mockResponse,
+    };
+
+    const mockErrorJson = safeStringifyException(mockError);
+    const spyLoggerFn = jest.spyOn(mockOptions.log, "error");
+    const spyStatus = jest.spyOn(mockResponse, "status");
+    const spyJson = jest.spyOn(mockResponse, "json");
+
+    await handleRestEndpointException({ ...mockOptions, error: mockError });
+
+    expect(spyStatus).toHaveBeenCalledWith(500);
+
+    expect(spyLoggerFn).toHaveBeenCalledWith(
+      mockOptions.errorMsg,
+      mockErrorJson,
+    );
+
+    expect(spyJson).toHaveBeenCalledOnce();
+
+    const mostRecentCall = spyJson.mock.lastCall;
+    expect(mostRecentCall).toBeTruthy();
+
+    expect(mostRecentCall?.[0].message).toBeString();
+    expect(mostRecentCall?.[0].message).toEqual("InternalServerError");
+    expect(mostRecentCall?.[0].error).toMatch(
+      /RuntimeError: Test error message(.*)An unexpected exception. Ha!/,
+    );
+  });
+
+  it("should escape malicious payloads in exception messages", async () => {
+    const mockResponse = createResponse();
+
+    const dummyXssPayload = `<script>stealAndUploadPrivateKeys();</script>`;
+    const mockError = new Error(dummyXssPayload);
+
+    const mockOptions: IHandleRestEndpointExceptionOptions = {
+      errorMsg: "Test error message",
+      log: jest.mocked(log), // Provide a mock logger if needed
+      error: mockError,
+      res: mockResponse,
+    };
+
+    const mockErrorJson = safeStringifyException(mockError);
+    const spyLoggerFn = jest.spyOn(mockOptions.log, "error");
+    const spyStatus = jest.spyOn(mockResponse, "status");
+    const spyJson = jest.spyOn(mockResponse, "json");
+
+    await handleRestEndpointException({ ...mockOptions, error: mockError });
+
+    expect(spyStatus).toHaveBeenCalledWith(500);
+
+    expect(spyLoggerFn).toHaveBeenCalledWith(
+      mockOptions.errorMsg,
+      mockErrorJson,
+    );
+
+    expect(spyJson).toHaveBeenCalledOnce();
+
+    const mostRecentCall = spyJson.mock.lastCall;
+    expect(mostRecentCall).toBeTruthy();
+
+    expect(mostRecentCall?.[0].message).toBeString();
+    expect(mostRecentCall?.[0].message).toEqual("InternalServerError");
+    expect(mostRecentCall?.[0].error).toMatch(
+      /RuntimeError: Test error message(.*)/,
+    );
+    expect(mostRecentCall?.[0].error).not.toMatch(
+      /.*stealAndUploadPrivateKeys.*/,
+    );
+  });
+
+  it("should escape malicious payloads in strings thrown", async () => {
+    const mockResponse = createResponse();
+
+    const dummyXssPayload = `<script>stealAndUploadPrivateKeys();</script>`;
+    const mockError = dummyXssPayload;
+
+    const mockOptions: IHandleRestEndpointExceptionOptions = {
+      errorMsg: "Test error message",
+      log: jest.mocked(log), // Provide a mock logger if needed
+      error: mockError,
+      res: mockResponse,
+    };
+
+    const mockErrorJson = safeStringifyException(mockError);
+    const spyLoggerFn = jest.spyOn(mockOptions.log, "error");
+    const spyStatus = jest.spyOn(mockResponse, "status");
+    const spyJson = jest.spyOn(mockResponse, "json");
+
+    await handleRestEndpointException({ ...mockOptions, error: mockError });
+
+    expect(spyStatus).toHaveBeenCalledWith(500);
+
+    expect(spyLoggerFn).toHaveBeenCalledWith(
+      mockOptions.errorMsg,
+      mockErrorJson,
+    );
+
+    expect(spyJson).toHaveBeenCalledOnce();
+
+    const mostRecentCall = spyJson.mock.lastCall;
+    expect(mostRecentCall).toBeTruthy();
+
+    expect(mostRecentCall?.[0].message).toBeString();
+    expect(mostRecentCall?.[0].message).toEqual("InternalServerError");
+    expect(mostRecentCall?.[0].error).toMatch(
+      /RuntimeError: Test error message(.*)/,
+    );
+    expect(mostRecentCall?.[0].error).not.toMatch(
+      /.*stealAndUploadPrivateKeys.*/,
+    );
+  });
+});

packages/cactus-plugin-keychain-memory/src/main/typescript/web-services/delete-keychain-entry-endpoint-v1.ts

@@ -87,6 +87,7 @@ export class DeleteKeychainEntryV1Endpoint implements IWebServiceEndpoint {
   }
 
   public async handleRequest(req: Request, res: Response): Promise<void> {
+    const { log } = this;
     const fnTag = `${this.className}#handleRequest()`;
     const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`;
     this.log.debug(reqTag);
@@ -96,7 +97,7 @@ export class DeleteKeychainEntryV1Endpoint implements IWebServiceEndpoint {
       res.json(resBody);
     } catch (ex) {
       const errorMsg = `${reqTag} ${fnTag} Failed to deploy contract:`;
-      handleRestEndpointException({ errorMsg, log: this.log, error: ex, res });
+      await handleRestEndpointException({ errorMsg, log, error: ex, res });
     }
   }
 }

packages/cactus-plugin-keychain-memory/src/main/typescript/web-services/get-keychain-entry-endpoint-v1.ts

@@ -87,6 +87,7 @@ export class GetKeychainEntryV1Endpoint implements IWebServiceEndpoint {
   }
 
   public async handleRequest(req: Request, res: Response): Promise<void> {
+    const { log } = this;
     const fnTag = `${this.className}#handleRequest()`;
     const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`;
     this.log.debug(reqTag);
@@ -96,7 +97,7 @@ export class GetKeychainEntryV1Endpoint implements IWebServiceEndpoint {
       res.json({ key, value });
     } catch (ex) {
       const errorMsg = `${reqTag} ${fnTag} Failed to deploy contract:`;
-      handleRestEndpointException({ errorMsg, log: this.log, error: ex, res });
+      await handleRestEndpointException({ errorMsg, log, error: ex, res });
     }
   }
 }

packages/cactus-plugin-keychain-memory/src/main/typescript/web-services/has-keychain-entry-endpoint-v1.ts

@@ -87,6 +87,7 @@ export class HasKeychainEntryV1Endpoint implements IWebServiceEndpoint {
   }
 
   public async handleRequest(req: Request, res: Response): Promise<void> {
+    const { log } = this;
     const fnTag = `${this.className}#handleRequest()`;
     const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`;
     this.log.debug(reqTag);
@@ -103,7 +104,7 @@ export class HasKeychainEntryV1Endpoint implements IWebServiceEndpoint {
       res.json(resBody);
     } catch (ex) {
       const errorMsg = `${reqTag} ${fnTag} Failed to deploy contract:`;
-      handleRestEndpointException({ errorMsg, log: this.log, error: ex, res });
+      await handleRestEndpointException({ errorMsg, log, error: ex, res });
     }
   }
 }

packages/cactus-plugin-keychain-memory/src/main/typescript/web-services/set-keychain-entry-endpoint-v1.ts

@@ -87,6 +87,7 @@ export class SetKeychainEntryV1Endpoint implements IWebServiceEndpoint {
   }
 
   public async handleRequest(req: Request, res: Response): Promise<void> {
+    const { log } = this;
     const fnTag = `${this.className}#handleRequest()`;
     const reqTag = `${this.getVerbLowerCase()} - ${this.getPath()}`;
     this.log.debug(reqTag);
@@ -96,7 +97,7 @@ export class SetKeychainEntryV1Endpoint implements IWebServiceEndpoint {
       res.json(resBody);
     } catch (ex) {
       const errorMsg = `${reqTag} ${fnTag} Failed to deploy contract:`;
-      handleRestEndpointException({ errorMsg, log: this.log, error: ex, res });
+      await handleRestEndpointException({ errorMsg, log, error: ex, res });
     }
   }
 }