From c1f730c69a4dbb0a6967f24b67b4e8998c8ef986 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Sat, 21 Feb 2026 01:47:33 -0800 Subject: [PATCH] fix: replace eval with declare and add base64 validation (#1557) * fix: replace eval with declare and add base64 validation (issues #1554, #1555) - shared/key-request.sh: replace eval with declare for defense-in-depth (eval avoided when safer declare alternative exists; validated vars stay safe) - fly/lib/common.sh: add base64 output alphabet validation before shell interpolation, matching daytona/lib/common.sh proven-safe pattern Fixes #1554 Fixes #1555 Agent: team-lead Co-Authored-By: Claude Sonnet 4.6 * fix: use printf -v instead of declare for safe variable assignment in key-request.sh Addresses security review feedback on PR #1557. The declare approach created a local variable whose export had no effect outside the function. printf -v assigns directly in the current scope without eval or command substitution. Agent: pr-maintainer Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 --- fly/lib/common.sh | 7 ++++++- shared/key-request.sh | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fly/lib/common.sh b/fly/lib/common.sh index d0e5a9a3..a30bbb9f 100644 --- a/fly/lib/common.sh +++ b/fly/lib/common.sh @@ -553,10 +553,15 @@ upload_file() { return 1 fi - # base64 output is safe (alphanumeric + /+=) so no injection risk local content content=$(base64 -w0 < "$local_path" 2>/dev/null || base64 < "$local_path") + # SECURITY: Validate base64 output contains only safe characters (defense-in-depth) + if [[ "${content}" =~ [^A-Za-z0-9+/=] ]]; then + log_error "upload_file: base64 output contains unexpected characters" + return 1 + fi + run_server "printf '%s' '${content}' | base64 -d > '${remote_path}'" } diff --git a/shared/key-request.sh b/shared/key-request.sh index 5ddcf840..3e16db9c 100644 --- a/shared/key-request.sh +++ b/shared/key-request.sh @@ -92,7 +92,8 @@ print(v) fi # SECURITY: val is already validated against ^[a-zA-Z0-9._/@-]+$ above, # and var_name is validated against ^[A-Z_][A-Z0-9_]*$ by the caller. - eval "${var_name}=\${val}" + # Use printf -v for safe variable assignment (no command substitution/expansion). + printf -v "${var_name}" '%s' "${val}" export "${var_name}" return 0 fi