Skip to content

fix: prevent command injection in WebAuthn/Push test LocalServers - #4251

Closed
harsh62 wants to merge 1 commit into
aws-amplify:mainfrom
harsh62:fix/localserver-command-injection
Closed

fix: prevent command injection in WebAuthn/Push test LocalServers#4251
harsh62 wants to merge 1 commit into
aws-amplify:mainfrom
harsh62:fix/localserver-command-injection

Conversation

@harsh62

@harsh62 harsh62 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Issue

The Node test utilities under AmplifyPlugins/.../LocalServer/index.mjs (used only to drive the iOS Simulator during WebAuthn and Push Notification UI tests) built shell command strings by interpolating request-body input and executed them with child_process.exec, which spawns /bin/sh.

For example:

const cmd = `xcrun simctl bootstatus ${deviceId} -b`
childProcess.exec(cmd, ...)

A deviceId such as booted; <command> posted to the localhost endpoints would therefore run arbitrary shell commands. The push server was additionally affected on /notifications, where the notification payload was piped through echo '<json>' | xcrun simctl push ... in a shell.

Impact is limited to developer machines that manually start these localhost-only test servers; no shipped SDK code is affected. Fixing it removes the footgun and keeps the test tooling safe.

Changes

Both test servers (AuthWebAuthnApp and PushNotificationHostApp):

  • Replace exec() with execFile(), passing arguments as an array so no shell is invoked and inputs are never interpreted as shell syntax.
  • /enroll: split the &&-chained command into two sequential execFile calls.
  • /notifications: write the APNS JSON payload to the child process's stdin instead of piping it through echo | ....
  • Add deviceId validation (UUID or booted) as defense-in-depth.

No dependency or package.json changes — execFile is part of core node:child_process.

Verification

Ran both servers locally against a stubbed xcrun that records its argv/stdin:

  • Valid deviceId (booted / UUID): commands run with the expected argument vector.
  • deviceId containing shell metacharacters: rejected with 400, no command executed.
  • Malicious title on /notifications: delivered verbatim as JSON via stdin, not executed.

A payload designed to touch /tmp/pwned produced no file in any case.

…Servers

The LocalServer test utilities built shell command strings by interpolating
the request-body `deviceId` (and, for push, notification fields) and ran
them via child_process.exec, which spawns /bin/sh. A crafted deviceId such
as "booted; <cmd>" sent to the localhost endpoints achieved arbitrary shell
command execution as the developer's user.

Fixes both test servers (AuthWebAuthnApp and PushNotificationHostApp):
- Replace exec() with execFile(), passing arguments as an array so no shell
  is invoked and user input is never interpreted as shell syntax.
- /enroll: split the '&&'-chained command into two sequential execFile calls.
- /notifications: write the APNS JSON payload to the process stdin instead of
  piping it through 'echo | ...' in a shell.
- Add UUID/"booted" validation on deviceId as defense-in-depth.

Impact is limited to developer workstations manually running these
localhost-only test servers; no shipped SDK code is affected.
@harsh62
harsh62 requested a review from a team as a code owner July 16, 2026 00:21
@harsh62

harsh62 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Closing to reopen from a branch on the main repository instead of a fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant