Fix infinite loop on EOF, eval injection risk, and subshell inefficiency

[ecrist]

Summary

Four bugs and efficiency issues found by static analysis and code review:

• [BUG - High] get_passphrase() infinite loops when stdin reaches EOF
• [BUG - Medium] set_var() passes insufficiently validated input to eval, allowing shell metacharacter injection
• [EFFICIENCY] build_ca() forks unnecessary subshells to read single-line passphrase temp files
• [EFFICIENCY/CLARITY] easyrsa_mktemp() uses opaque nested loops that hide the 20-slot temp file limit

---

Detail

1. Infinite loop on EOF — hide_read_pass() / get_passphrase()

hide_read_pass() called read -r in each branch but discarded its exit status, always returning 0. When stdin reaches EOF (read returns 1 — e.g. non-interactive use, piped input, closed terminal), the caller get_passphrase() never detected the failure. Because r stays empty, ${#r} < 4 is true, the "too short" message is printed, and the while : loop restarts — indefinitely.

The return 1 at the end of get_passphrase() (after done) was unreachable dead code.

Fix: Capture read's exit status in every branch of hide_read_pass() and return $ret. In get_passphrase(), add || return 1 to the hide_read_pass call so EOF propagates cleanly to the caller.

# hide_read_pass — each branch now:
read -r "$@"
ret=$?
# ... restore echo state ...
return $ret

# get_passphrase — break out of infinite loop on EOF:
hide_read_pass r || return 1

---

2. set_var() — incomplete validation before eval

The existing guard only rejected names containing =:

case "$1" in
    *=*) user_error "set_var - var '$1'"
esac
eval "export \"$1\"=\"\${$1-$2}\""

Names containing backticks, $(), semicolons, spaces, or other shell metacharacters passed the check and were handed directly to eval.

Fix: Restrict $1 to valid POSIX identifier characters — alphanumeric plus underscore, not starting with a digit — and reject empty strings:

case "$1" in
    ''|*=*|*[!A-Za-z0-9_]*|[0-9]*) user_error "set_var - var '$1'"
esac

No legitimate internal call site is affected; all existing callers pass known-good variable names.

---

3. Unnecessary subshells reading passphrase temp files — build_ca()

# Before — forks a subshell + external cat process per line:
p="$(cat "$in_key_pass_tmp")"
q="$(cat "$out_key_pass_tmp")"

# After — no fork, no external process:
read -r p < "$in_key_pass_tmp"
read -r q < "$out_key_pass_tmp"

get_passphrase() writes passphrases with printf '%s' (no trailing newline); the value is always a single line. read -r and $(cat ...) are semantically equivalent here. ($(...) strips trailing newlines, but there are none to strip.)

---

4. Opaque nested loop in easyrsa_mktemp()

The slot allocator used two nested for loops over literal word lists to produce temp.00–temp.19. The hard limit of 20 slots was invisible and required editing two separate loop heads to change.

# Before:
for high in 0 1; do
    for low in 0 1 2 3 4 5 6 7 8 9; do
        shotfile="${secured_session}/temp.${high}${low}"
        ...

# After — limit is explicit and in one place:
i=0
while [ "$i" -lt 20 ]; do
    shotfile="${secured_session}/temp.$(printf '%02d' "$i")"
    i=$((i + 1))
    ...

Slot names (temp.00–temp.19) are identical; no existing temp files or tests are affected.

---

Test plan

A regression test script (unit-tests-pr1436.sh) is included in this PR and wired into op-test.sh via the new run_local_regression_tests() function. It runs automatically alongside the existing downloaded unit tests with no external dependencies.

The seven tests and what they cover:

Test	What it validates
FIX-1-passphrase-eof-exits	build-ca with /dev/null stdin exits within 10 s — confirms no infinite loop
FIX-2a-set_var-rejects-digit-leading-name	set_var 0INVALID … in vars file → non-zero exit
FIX-2b-set_var-rejects-hyphen-in-name	set_var EASYRSA-INVALID … in vars file → non-zero exit
FIX-2c-set_var-accepts-valid-identifier	set_var EASYRSA_REQ_CN … still works (regression guard)
FIX-2d-set_var-rejects-empty-name	Empty variable name → non-zero exit
FIX-3-passphrase-comparison-regression	build-ca with --passout/--passin succeeds (read -r parity check)
FIX-4-easyrsa-mktemp-naming	--keep-tmp snapshot contains temp.00 with counter-loop naming

Run locally: sh unit-tests-pr1436.sh -v

All 7 tests pass on the current branch.

🤖 Generated with Claude Code