Skip to content

fix(config): the JS/TS config loader interpolates package names and paths into a JS template string without escaping #802

Description

@BryanFRD

loader_js.rs builds the loader script by string formatting, and several interpolated values are neither escaped nor validated.

reifyHooks (src/config/loader_js.rs:44):

const cmd = `${runtime} --input-type=module -e "const m = await import('${fileUrl}'); ... const hooks = ${hookPath}; await hooks.${key}(${ctx});"`;

and the caller (:65):

pkg.hooks = reifyHooks(pkg.hooks, '{file_url}', '{runtime}', `cfg.package.find(p=>p.name==="${pkg.name}").hooks`);

Three injection points, all reached from values the user controls in their own config:

  • pkg.name lands inside a double-quoted JS string. A package named a"]);evil();// breaks out of it. Package names come from ferrflow.js, and in monorepo auto-discovery flows from workspace package.json files — which can arrive via a dependency-managed workspace, so "the user's own config" is not always literally true.
  • key (the hook name) is spliced as a bare property access: await hooks.${key}(…). A hook key that is not a valid identifier produces a syntax error at hook time rather than a config error at load time.
  • file_url goes into a single-quoted JS string, which then sits inside a double-quoted shell argument. A repository path containing ', " or $ breaks the quoting. On Unix, $(…) in a path is command substitution when the reified command is later run through sh -c by the hook runner. Paths with quotes are legal on Unix and not that rare on shared build machines.

Realistically this is a robustness problem more than an attack — the primary consequence is confusing breakage from a path or package name that happens to contain a quote. But the reified string ends up executed by sh -c, so it is worth closing properly rather than documenting.

Fix

Stop building JS by concatenation. Pass the values as data:

  • Emit the package name, hook path and file URL via JSON.stringify on the Rust side (serde_json::to_string produces valid JS string literals) rather than raw {} interpolation.
  • Look hooks up with hooks[key] instead of hooks.${key}, so any string key works and no identifier validity is assumed.
  • Better still: hand the loader its inputs through argv or an env var containing one JSON blob, and have the fixed loader script read them. Then the script is a constant and nothing user-controlled is ever parsed as code.

Validate package names at config-load time as well — a name that is not [A-Za-z0-9._@/-]+ should be a clear config error, not a downstream syntax error.

Tests

  • A ferrflow.js declaring a package named a"]);throw new Error('pwned');// with a function hook: loading the config must either reject the name or produce a working reified command, never execute the injected fragment.
  • A repo checked out at a path containing a single quote loads a .ts config successfully.
  • A hook key that is not a valid JS identifier ("post-bump") reifies and runs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Low priority / somedaybugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions