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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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