Skip to content

Minor correctness/robustness fixes: empty-string override, secret logging, version precedence, ZIP bounds #267

Description

@aosmcleod

Summary

A bundle of smaller, verified correctness/robustness issues found during an audit. Grouped because each is low severity; happy to split into separate issues if preferred.

1. Empty-string platform override silently ignored — src/shared/config.ts:122

result.command = platformConfig.command || result.command uses ||, so a deliberate empty-string override (command: "") is falsy and falls back to the base command. Reproduced: base command:"default", darwin override command:"" → resolved "default" (expected ""). Fix: use !== undefined presence checks.

2. replaceVariables logs raw config values (potential secret leak) — src/shared/config.ts:32-35

When a replacement is an array used in string context, the code does console.warn(..., { key, replacement }), printing the raw value. user_config options can be sensitive: true (API keys, passwords), and this path uses bare console.warn (not the silenceable logger). Fix: log the key/var name only; never the value.

3. manifest_version vs deprecated dxt_version precedence — src/shared/manifestVersionResolve.ts:5-22

If manifest_version is present but unsupported (e.g. "0.5") while a supported dxt_version (e.g. "0.3") is also present, the authoritative newer field is ignored and resolution falls through to the deprecated dxt_version. Mostly self-correcting (strict schema rejects later), but produces a confusing error. Fix: if manifest_version is present but unsupported, error on that rather than silently using dxt_version.

4. Malformed ZIP central directory → uncaught RangeError region — src/cli/unpack.ts:53-84

The EOCD scan and central-directory walk call readUInt32LE/readUInt16LE/toString at offsets derived from untrusted ZIP fields with no bounds validation against zipBuffer.length. A crafted centralDirOffset throws RangeError; it's caught by the outer handler but silently skips the permissions pass for the whole archive. Fix: validate eocdOffset + 16 <= len, centralDirOffset < len, and offset + 46 + filenameLength <= len before each read.

Environment: current main (70fe3b3).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions