fix: escape */ to prevent premature doc comment termination (#102)

As a security measure, we want to escape the JSDoc closing char sequence
(`*/`) to prevent unwanted code to be injected in the generated
bindings.

Author: ilbertt

src/core/generate/rs/src/bindings/comments.rs

@@ -0,0 +1,5 @@
+/// Escapes doc comment content to prevent comment injection attacks.
+/// Replaces `*/` with `*\/` to prevent premature comment termination.
+pub(super) fn escape_doc_comment(line: &str) -> String {
+    line.replace("*/", r"*\/")
+}

src/core/generate/rs/src/bindings/mod.rs

@@ -1,3 +1,4 @@
+mod comments;
 pub mod javascript;
 pub mod typescript;
 pub mod typescript_native;

src/core/generate/rs/src/bindings/typescript.rs

@@ -1,5 +1,6 @@
 //! Ported from https://github.com/dfinity/candid/blob/1ddf879f368f765145223c08bbe2c8c8f4782dcc/rust/candid_parser/src/bindings/typescript.rs
 
+use super::comments::escape_doc_comment;
 use super::javascript::{ident, is_tuple_fields};
 use candid::pretty::utils::*;
 use candid::types::{Field, Function, Label, SharedLabel, Type, TypeEnv, TypeInner};
@@ -250,10 +251,9 @@ fn pp_docs<'a>(docs: &'a [String]) -> RcDoc<'a> {
     if docs.is_empty() {
         RcDoc::nil()
     } else {
-        let docs = lines(
-            docs.iter()
-                .map(|line| RcDoc::text(DOC_COMMENT_LINE_PREFIX).append(line)),
-        );
+        let docs = lines(docs.iter().map(|line| {
+            RcDoc::text(DOC_COMMENT_LINE_PREFIX).append(RcDoc::text(escape_doc_comment(line)))
+        }));
         RcDoc::text(DOC_COMMENT_PREFIX)
             .append(RcDoc::hardline())
             .append(docs)

src/core/generate/rs/src/bindings/typescript_native/comments.rs

@@ -1,4 +1,5 @@
 use super::conversion_functions_generator::TopLevelNodes;
+use crate::bindings::comments::escape_doc_comment;
 use swc_core::common::{
     BytePos, DUMMY_SP, Span,
     comments::{Comment, CommentKind, Comments},
@@ -26,7 +27,7 @@ fn make_comment(docs: &[String]) -> Option<Comment> {
         // Join all doc lines into a single block comment, with each line prefixed by a space
         let mut comment_text = String::from("*\n");
         for line in docs {
-            comment_text.push_str(&format!(" * {}\n", line));
+            comment_text.push_str(&format!(" * {}\n", escape_doc_comment(line)));
         }
 
         // Add a space at the end to align the block comment final line ("*/") properly

tests/assets/malicious_doc.did

@@ -0,0 +1,13 @@
+// */ import { malicious } from 'attacker'; console.log('injected!'); /*
+// Normal doc line
+// Another */ attempt /* to inject
+type MaliciousType = record {
+    // Doc comment for field with */ malicious code /* in it
+    field: text;
+};
+
+// Service with */ malicious */ doc
+service : {
+    // Method with */ in doc comment
+    get : () -> (MaliciousType) query;
+}

tests/security.test.ts

@@ -0,0 +1,40 @@
+import { beforeAll, describe, expect, it } from 'vitest';
+import { wasmGenerate } from '../src/core/generate/rs.ts';
+import { testWasmInit } from './utils/wasm.ts';
+
+const TESTS_ASSETS_DIR = './tests/assets';
+const SNAPSHOTS_BASE_DIR = './snapshots/wasm-generate';
+const SERVICE_NAME = 'malicious_doc';
+
+beforeAll(async () => {
+  await testWasmInit();
+});
+
+describe('security', () => {
+  describe('doc comment injection prevention', () => {
+    it('should escape */ in doc comments to prevent code injection', async () => {
+      const didFile = `${TESTS_ASSETS_DIR}/${SERVICE_NAME}.did`;
+
+      const result = wasmGenerate({
+        did_file_path: didFile,
+        service_name: SERVICE_NAME,
+        declarations: {
+          root_exports: true,
+        },
+      });
+
+      await expect(result.declarations_js).toMatchFileSnapshot(
+        `${SNAPSHOTS_BASE_DIR}/${SERVICE_NAME}/declarations/${SERVICE_NAME}.did.js.snapshot`,
+      );
+      await expect(result.declarations_ts).toMatchFileSnapshot(
+        `${SNAPSHOTS_BASE_DIR}/${SERVICE_NAME}/declarations/${SERVICE_NAME}.did.d.ts.snapshot`,
+      );
+      await expect(result.interface_ts).toMatchFileSnapshot(
+        `${SNAPSHOTS_BASE_DIR}/${SERVICE_NAME}/${SERVICE_NAME}.d.ts.snapshot`,
+      );
+      await expect(result.service_ts).toMatchFileSnapshot(
+        `${SNAPSHOTS_BASE_DIR}/${SERVICE_NAME}/${SERVICE_NAME}.ts.snapshot`,
+      );
+    });
+  });
+});

tests/snapshots/wasm-generate/malicious_doc/declarations/malicious_doc.did.d.ts.snapshot

@@ -0,0 +1,28 @@
+import type { ActorMethod } from '@icp-sdk/core/agent';
+import type { IDL } from '@icp-sdk/core/candid';
+import type { Principal } from '@icp-sdk/core/principal';
+
+/**
+ * *\/ import { malicious } from 'attacker'; console.log('injected!'); /*
+ * Normal doc line
+ * Another *\/ attempt /* to inject
+ */
+export interface MaliciousType {
+  /**
+   * Doc comment for field with *\/ malicious code /* in it
+   */
+  'field' : string,
+}
+/**
+ * Service with *\/ malicious *\/ doc
+ */
+export interface _SERVICE {
+  /**
+   * Method with *\/ in doc comment
+   */
+  'get' : ActorMethod<[], MaliciousType>,
+}
+export declare const idlService: IDL.ServiceClass;
+export declare const idlInitArgs: IDL.Type[];
+export declare const idlFactory: IDL.InterfaceFactory;
+export declare const init: (args: { IDL: typeof IDL }) => IDL.Type[];

tests/snapshots/wasm-generate/malicious_doc/declarations/malicious_doc.did.js.snapshot

@@ -0,0 +1,17 @@
+import { IDL } from '@icp-sdk/core/candid';
+
+export const MaliciousType = IDL.Record({ 'field' : IDL.Text });
+
+export const idlService = IDL.Service({
+  'get' : IDL.Func([], [MaliciousType], ['query']),
+});
+
+export const idlInitArgs = [];
+
+export const idlFactory = ({ IDL }) => {
+  const MaliciousType = IDL.Record({ 'field' : IDL.Text });
+  
+  return IDL.Service({ 'get' : IDL.Func([], [MaliciousType], ['query']) });
+};
+
+export const init = ({ IDL }) => { return []; };

tests/snapshots/wasm-generate/malicious_doc/malicious_doc.d.ts.snapshot

@@ -0,0 +1,32 @@
+import { type HttpAgentOptions, type ActorConfig, type Agent } from "@icp-sdk/core/agent";
+import type { Principal } from "@icp-sdk/core/principal";
+import { type _SERVICE } from "./declarations/malicious_doc.did";
+export interface Some<T> {
+    __kind__: "Some";
+    value: T;
+}
+export interface None {
+    __kind__: "None";
+}
+export type Option<T> = Some<T> | None;
+export interface MaliciousType {
+    /**
+     * Doc comment for field with *\/ malicious code /* in it
+     */
+    field: string;
+}
+/**
+ * Service with *\/ malicious *\/ doc
+ */
+export interface malicious_docInterface {
+    /**
+     * Method with *\/ in doc comment
+     */
+    get(): Promise<MaliciousType>;
+}
+export interface CreateActorOptions {
+    agent?: Agent;
+    agentOptions?: HttpAgentOptions;
+    actorOptions?: ActorConfig;
+}
+export declare function createActor(canisterId: string, options: CreateActorOptions = {}): malicious_docInterface;

tests/snapshots/wasm-generate/malicious_doc/malicious_doc.ts.snapshot

@@ -0,0 +1,86 @@
+import { Actor, HttpAgent, type HttpAgentOptions, type ActorConfig, type Agent, type ActorSubclass } from "@icp-sdk/core/agent";
+import type { Principal } from "@icp-sdk/core/principal";
+import { idlFactory, type _SERVICE } from "./declarations/malicious_doc.did";
+export interface Some<T> {
+    __kind__: "Some";
+    value: T;
+}
+export interface None {
+    __kind__: "None";
+}
+export type Option<T> = Some<T> | None;
+function some<T>(value: T): Some<T> {
+    return {
+        __kind__: "Some",
+        value: value
+    };
+}
+function none(): None {
+    return {
+        __kind__: "None"
+    };
+}
+function isNone<T>(option: Option<T>): option is None {
+    return option.__kind__ === "None";
+}
+function isSome<T>(option: Option<T>): option is Some<T> {
+    return option.__kind__ === "Some";
+}
+function unwrap<T>(option: Option<T>): T {
+    if (isNone(option)) {
+        throw new Error("unwrap: none");
+    }
+    return option.value;
+}
+function candid_some<T>(value: T): [T] {
+    return [
+        value
+    ];
+}
+function candid_none<T>(): [] {
+    return [];
+}
+function record_opt_to_undefined<T>(arg: T | null): T | undefined {
+    return arg == null ? undefined : arg;
+}
+export interface MaliciousType {
+    /**
+     * Doc comment for field with *\/ malicious code /* in it
+     */
+    field: string;
+}
+/**
+ * Service with *\/ malicious *\/ doc
+ */
+export interface malicious_docInterface {
+    /**
+     * Method with *\/ in doc comment
+     */
+    get(): Promise<MaliciousType>;
+}
+export class Malicious_doc implements malicious_docInterface {
+    constructor(private actor: ActorSubclass<_SERVICE>){}
+    async get(): Promise<MaliciousType> {
+        const result = await this.actor.get();
+        return result;
+    }
+}
+export interface CreateActorOptions {
+    agent?: Agent;
+    agentOptions?: HttpAgentOptions;
+    actorOptions?: ActorConfig;
+}
+export function createActor(canisterId: string, options: CreateActorOptions = {}): Malicious_doc {
+    const agent = options.agent || HttpAgent.createSync({
+        ...options.agentOptions
+    });
+    if (options.agent && options.agentOptions) {
+        console.warn("Detected both agent and agentOptions passed to createActor. Ignoring agentOptions and proceeding with the provided agent.");
+    }
+    const actor = Actor.createActor<_SERVICE>(idlFactory, {
+        agent,
+        canisterId: canisterId,
+        ...options.actorOptions
+    });
+    return new Malicious_doc(actor);
+}