[Detail Bug] Bash: eval escaping context is lost for deferred-rendered fragments

[detail-app]

Summary

• Context: The TranslateModule trait defines how syntax modules are converted into intermediate code fragments (FragmentKind) during the translation phase.
• Bug: The translate_eval method temporarily sets the eval_ctx flag in TranslateMetadata during translation but fails to persist this state in the returned fragments.
• Actual vs. expected: Currently, the eval_ctx is restored to its original value before the fragment is returned, causing the fragment to be rendered later without the necessary eval escaping; it should instead ensure that the returned fragment tree is aware of the intended eval context.
• Impact: Inconsistent escaping for Bash eval commands, leading to broken scripts and potential security vulnerabilities due to premature variable expansion.

Code with bug

    fn translate_eval(&self, meta: &mut TranslateMetadata, is_eval: bool) -> FragmentKind {
        let prev = meta.eval_ctx;
        meta.eval_ctx = is_eval;
        let expr = self.translate(meta); // <-- BUG 🔴 [eval_ctx is set here but ignored by most fragments]
        meta.eval_ctx = prev; // <-- BUG 🔴 [eval_ctx is restored before rendering, losing the escaping context]
        expr
    }

Evidence

Inconsistent behavior was confirmed by comparing a RawFragment (which captures context during translation) with a VarExprFragment (which defers it until rendering). When both are translated via translate_eval(meta, true) and then rendered while eval_ctx is false:

• RawFragment result: \$foo (escaped)
• VarExprFragment result: "${foo}" (not escaped)

This inconsistency proves that translate_eval fails to provide a uniform context for the resulting fragment tree. The VarExprFragment "loses" the information that it was supposed to be for an eval context because translate_eval restores the metadata state before the fragment is ever rendered.

Exploit scenario (security bug)

If a variable expansion is intended to be protected by eval escaping but the escaping is lost due to this bug, the eval command will expand the variable prematurely. If the variable's value is untrusted and contains Bash metacharacters (e.g., ; rm -rf /), it can lead to arbitrary command execution. For example, eval "var=\"$untrusted\"" is unsafe, whereas eval "var=\"\$untrusted\"" is safe because it defers expansion to the subshell.

Why has this bug gone undetected?

Many callers happen to be inside other fragments (like VarStmtFragment) that manually re-set the eval_ctx during their own rendering phase. This masks the fact that translate_eval itself is ineffective. However, this creates a significant risk for any new code or existing code that relies on translate_eval for non-statement fragments.

Recommended fix

The TranslateModule::translate_eval method should be redesigned to ensure context persistence, or removed if context should only be managed during the rendering phase. If it remains, fragments should likely store the eval_ctx state they were translated with. Additionally, eval_ctx in TranslateMetadata should be marked with #[context] to allow for safer automated management.

History

This bug was introduced in commit 4b15d0. This commit refactored the translation architecture to use deferred rendering with FragmentKind, but TranslateModule::translate_eval continued to treat eval_ctx as a transient state that is restored before the returned fragment is actually rendered.