Skip to content

Commit eb53a4f

Browse files
joshuayoesjoshuayoes
authored
Merge commit from fork
chore: add command injection protections, document in README.md and SECURITY.md
2 parents 042fdb7 + 73a78fb commit eb53a4f

File tree

5 files changed

+187
-46
lines changed

5 files changed

+187
-46
lines changed

‎QA.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Quality Assurance
2+
3+
This guide contains manual quality assurance tests to make sure all the tools in this MCP server is functional on release.
4+
5+
You can run a test case copy and pasting the test case into a chat in an MCP client (like Cursor) that can run MCP tools.
6+
7+
## Test Case: Photos app
8+
9+
1. Have the user open the native Photo app in the iOS simulator.
10+
2. Call `get_booted_sim_id` to get the UDID of the booted simulator.
11+
3. Call `record_video` to start recording a screen recording of the test.
12+
4. Call `ui_describe_all` to make sure we are on the All Photos tab.
13+
5. Call `ui_describe_point` to find the x and y coordinates for tapping the Search tab button.
14+
6. Call `ui_tap` to tap the Search tab button.
15+
7. Call `ui_tap` to focus on the Search text input.
16+
8. Call `ui_type` to type "Photos" into the Search text input.
17+
9. Call `ui_describe_all` to describe the page and find the first photo result.
18+
10. Call `ui_describe_point` to find the x and y coordinates for the first photo result touchable area.
19+
11. Call `ui_tap` to tap the coordinates of the first photo result touchable area
20+
12. Call `ui_swipe` to swipe from the center of the screen down to dismiss the photo and go back to the All Photos tab.
21+
13. Call `ui_describe_all` to describe the page and see we are the All Photos tab.
22+
14. Call `screenshot` to take a screenshot of the current page.
23+
15. Call `stop_recording` to stop the screen recording.

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
A Model Context Protocol (MCP) server for interacting with iOS simulators. This server allows you to interact with iOS simulators by getting information about them, controlling UI interactions, and inspecting UI elements.
66

7+
> **Security Notice**: Command injection vulnerabilities present in versions < 1.3.3 have been fixed. Please update to v1.3.3 or later. See [SECURITY.md](SECURITY.md) for details.
8+
79
<a href="https://glama.ai/mcp/servers/@joshuayoes/ios-simulator-mcp">
810
<img width="380" height="200" src="https://glama.ai/mcp/servers/@joshuayoes/ios-simulator-mcp/badge" alt="iOS Simulator MCP server" />
911
</a>

‎SECURITY.md‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,24 @@
55
Use this section to tell people about which versions of your project are
66
currently being supported with security updates.
77

8-
| Version | Supported |
9-
| ------- | ------------------ |
10-
| * | :white_check_mark: |
8+
| Version | Supported |
9+
| -------- | ------------------ |
10+
| >= 1.3.3 | :white_check_mark: |
11+
| < 1.3.3 | :x: |
12+
13+
## Fixed Vulnerabilities
14+
15+
### Command Injection (Fixed in v1.3.3)
16+
17+
**CVE**: To be assigned
18+
**Severity**: Moderate
19+
**Fixed in**: v1.3.3 (2025)
20+
21+
**Description**: Previous versions contained command injection vulnerabilities in several MCP tools (ui_tap, ui_type, ui_swipe, ui_describe_point, ui_describe_all, screenshot, record_video, stop_recording) due to unsafe shell command construction using string interpolation.
22+
23+
**Impact**: Malicious input could potentially execute arbitrary commands on the host system.
24+
25+
**Fix**: Replaced unsafe `execAsync` string interpolation with secure `execFile` calls using argument arrays. Added input validation.
1126

1227
## Reporting a Vulnerability
1328

@@ -16,10 +31,12 @@ To report a security issue, please use the GitHub Security Advisory "Report a Vu
1631
You can expect an initial response to your report within 48 hours. We will keep you informed about the progress of addressing the vulnerability and will work with you to coordinate the disclosure timeline.
1732

1833
If the vulnerability is accepted:
34+
1935
- We will work on a fix and keep you updated on the progress
2036
- Once a fix is ready, we will coordinate with you on the disclosure timeline
2137
- You will be credited for the discovery (unless you prefer to remain anonymous)
2238

2339
If the vulnerability is declined:
40+
2441
- We will provide a detailed explanation of why it was not accepted
25-
- If appropriate, we will suggest alternative approaches or mitigations
42+
- If appropriate, we will suggest alternative approaches or mitigations

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "ios-simulator-mcp",
3-
"version": "1.3.2",
3+
"version": "1.3.3",
44
"description": "MCP server for interacting with the iOS simulator",
55
"bin": {
66
"ios-simulator-mcp": "./build/index.js"

‎src/index.ts‎

Lines changed: 140 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,36 @@
22

33
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
44
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
5-
import { exec, spawn } from "child_process";
5+
import { execFile, spawn } from "child_process";
66
import { promisify } from "util";
77
import { z } from "zod";
88
import path from "path";
99
import os from "os";
1010

11-
const execAsync = promisify(exec);
11+
const execFileAsync = promisify(execFile);
12+
13+
/**
14+
* Strict UDID/UUID pattern: 8-4-4-4-12 hexadecimal characters (e.g. 37A360EC-75F9-4AEC-8EFA-10F4A58D8CCA)
15+
*/
16+
const UDID_REGEX =
17+
/^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$/;
18+
19+
/**
20+
* Runs a command with arguments and returns the stdout and stderr
21+
* @param cmd - The command to run
22+
* @param args - The arguments to pass to the command
23+
* @returns The stdout and stderr of the command
24+
*/
25+
async function run(
26+
cmd: string,
27+
args: string[]
28+
): Promise<{ stdout: string; stderr: string }> {
29+
const { stdout, stderr } = await execFileAsync(cmd, args, { shell: false });
30+
return {
31+
stdout: stdout.trim(),
32+
stderr: stderr.trim(),
33+
};
34+
}
1235

1336
// Read filtered tools from environment variable
1437
const FILTERED_TOOLS =
@@ -49,7 +72,7 @@ function errorWithTroubleshooting(message: string): string {
4972
}
5073

5174
async function getBootedDevice() {
52-
const { stdout, stderr } = await execAsync("xcrun simctl list devices");
75+
const { stdout, stderr } = await run("xcrun", ["simctl", "list", "devices"]);
5376

5477
if (stderr) throw new Error(stderr);
5578

@@ -130,16 +153,22 @@ if (!isToolFiltered("ui_describe_all")) {
130153
{
131154
udid: z
132155
.string()
156+
.regex(UDID_REGEX)
133157
.optional()
134158
.describe("Udid of target, can also be set with the IDB_UDID env var"),
135159
},
136160
async ({ udid }) => {
137161
try {
138162
const actualUdid = await getBootedDeviceId(udid);
139163

140-
const { stdout } = await execAsync(
141-
`idb ui describe-all --udid ${actualUdid} --json --nested`
142-
);
164+
const { stdout } = await run("idb", [
165+
"ui",
166+
"describe-all",
167+
"--udid",
168+
actualUdid,
169+
"--json",
170+
"--nested",
171+
]);
143172

144173
return {
145174
isError: false,
@@ -167,9 +196,14 @@ if (!isToolFiltered("ui_tap")) {
167196
"ui_tap",
168197
"Tap on the screen in the iOS Simulator",
169198
{
170-
duration: z.string().optional().describe("Press duration"),
199+
duration: z
200+
.string()
201+
.regex(/^\d+(\.\d+)?$/)
202+
.optional()
203+
.describe("Press duration"),
171204
udid: z
172205
.string()
206+
.regex(UDID_REGEX)
173207
.optional()
174208
.describe("Udid of target, can also be set with the IDB_UDID env var"),
175209
x: z.number().describe("The x-coordinate"),
@@ -178,10 +212,21 @@ if (!isToolFiltered("ui_tap")) {
178212
async ({ duration, udid, x, y }) => {
179213
try {
180214
const actualUdid = await getBootedDeviceId(udid);
181-
const durationArg = duration ? `--duration ${duration}` : "";
182-
const { stderr } = await execAsync(
183-
`idb ui tap --udid ${actualUdid} ${durationArg} ${x} ${y} --json`
184-
);
215+
216+
const { stderr } = await run("idb", [
217+
"ui",
218+
"tap",
219+
"--udid",
220+
actualUdid,
221+
...(duration ? ["--duration", duration] : []),
222+
"--json",
223+
// When passing user-provided values to a command, it's crucial to use `--`
224+
// to separate the command's options from positional arguments.
225+
// This prevents the shell from misinterpreting the arguments as options.
226+
"--",
227+
String(x),
228+
String(y),
229+
]);
185230

186231
if (stderr) throw new Error(stderr);
187232

@@ -213,16 +258,30 @@ if (!isToolFiltered("ui_type")) {
213258
{
214259
udid: z
215260
.string()
261+
.regex(UDID_REGEX)
216262
.optional()
217263
.describe("Udid of target, can also be set with the IDB_UDID env var"),
218-
text: z.string().describe("Text to input"),
264+
text: z
265+
.string()
266+
.max(500)
267+
.regex(/^[\x20-\x7E]+$/)
268+
.describe("Text to input"),
219269
},
220270
async ({ udid, text }) => {
221271
try {
222272
const actualUdid = await getBootedDeviceId(udid);
223-
const { stderr } = await execAsync(
224-
`idb ui text ${text} --udid ${actualUdid}`
225-
);
273+
274+
const { stderr } = await run("idb", [
275+
"ui",
276+
"text",
277+
"--udid",
278+
actualUdid,
279+
// When passing user-provided values to a command, it's crucial to use `--`
280+
// to separate the command's options from positional arguments.
281+
// This prevents the shell from misinterpreting the arguments as options.
282+
"--",
283+
text,
284+
]);
226285

227286
if (stderr) throw new Error(stderr);
228287

@@ -256,6 +315,7 @@ if (!isToolFiltered("ui_swipe")) {
256315
{
257316
udid: z
258317
.string()
318+
.regex(UDID_REGEX)
259319
.optional()
260320
.describe("Udid of target, can also be set with the IDB_UDID env var"),
261321
x_start: z.number().describe("The starting x-coordinate"),
@@ -271,10 +331,23 @@ if (!isToolFiltered("ui_swipe")) {
271331
async ({ udid, x_start, y_start, x_end, y_end, delta }) => {
272332
try {
273333
const actualUdid = await getBootedDeviceId(udid);
274-
const deltaArg = delta ? `--delta ${delta}` : "";
275-
const { stderr } = await execAsync(
276-
`idb ui swipe --udid ${actualUdid} ${deltaArg} ${x_start} ${y_start} ${x_end} ${y_end} --json`
277-
);
334+
335+
const { stderr } = await run("idb", [
336+
"ui",
337+
"swipe",
338+
"--udid",
339+
actualUdid,
340+
...(delta ? ["--delta", String(delta)] : []),
341+
"--json",
342+
// When passing user-provided values to a command, it's crucial to use `--`
343+
// to separate the command's options from positional arguments.
344+
// This prevents the shell from misinterpreting the arguments as options.
345+
"--",
346+
String(x_start),
347+
String(y_start),
348+
String(x_end),
349+
String(y_end),
350+
]);
278351

279352
if (stderr) throw new Error(stderr);
280353

@@ -306,6 +379,7 @@ if (!isToolFiltered("ui_describe_point")) {
306379
{
307380
udid: z
308381
.string()
382+
.regex(UDID_REGEX)
309383
.optional()
310384
.describe("Udid of target, can also be set with the IDB_UDID env var"),
311385
x: z.number().describe("The x-coordinate"),
@@ -314,9 +388,20 @@ if (!isToolFiltered("ui_describe_point")) {
314388
async ({ udid, x, y }) => {
315389
try {
316390
const actualUdid = await getBootedDeviceId(udid);
317-
const { stdout, stderr } = await execAsync(
318-
`idb ui describe-point --udid ${actualUdid} ${x} ${y} --json`
319-
);
391+
392+
const { stdout, stderr } = await run("idb", [
393+
"ui",
394+
"describe-point",
395+
"--udid",
396+
actualUdid,
397+
"--json",
398+
// When passing user-provided values to a command, it's crucial to use `--`
399+
// to separate the command's options from positional arguments.
400+
// This prevents the shell from misinterpreting the arguments as options.
401+
"--",
402+
String(x),
403+
String(y),
404+
]);
320405

321406
if (stderr) throw new Error(stderr);
322407

@@ -362,10 +447,12 @@ if (!isToolFiltered("screenshot")) {
362447
{
363448
udid: z
364449
.string()
450+
.regex(UDID_REGEX)
365451
.optional()
366452
.describe("Udid of target, can also be set with the IDB_UDID env var"),
367453
output_path: z
368454
.string()
455+
.max(1024)
369456
.describe(
370457
"File path where the screenshot will be saved (if relative, ~/Downloads will be used as base directory)"
371458
),
@@ -393,14 +480,21 @@ if (!isToolFiltered("screenshot")) {
393480
const actualUdid = await getBootedDeviceId(udid);
394481
const absolutePath = ensureAbsolutePath(output_path);
395482

396-
let command = `xcrun simctl io ${actualUdid} screenshot ${absolutePath}`;
397-
398-
if (type) command += ` --type=${type}`;
399-
if (display) command += ` --display=${display}`;
400-
if (mask) command += ` --mask=${mask}`;
401-
402483
// command is weird, it responds with stderr on success and stdout is blank
403-
const { stderr: stdout } = await execAsync(command);
484+
const { stderr: stdout } = await run("xcrun", [
485+
"simctl",
486+
"io",
487+
actualUdid,
488+
"screenshot",
489+
...(type ? [`--type=${type}`] : []),
490+
...(display ? [`--display=${display}`] : []),
491+
...(mask ? [`--mask=${mask}`] : []),
492+
// When passing user-provided values to a command, it's crucial to use `--`
493+
// to separate the command's options from positional arguments.
494+
// This prevents the shell from misinterpreting the arguments as options.
495+
"--",
496+
absolutePath,
497+
]);
404498

405499
// throw if we don't get the expected success message
406500
if (stdout && !stdout.includes("Wrote screenshot to")) {
@@ -440,6 +534,7 @@ if (!isToolFiltered("record_video")) {
440534
{
441535
output_path: z
442536
.string()
537+
.max(1024)
443538
.optional()
444539
.describe(
445540
`Optional output path (defaults to ~/Downloads/simulator_recording_$DATE.mp4)`
@@ -474,18 +569,22 @@ if (!isToolFiltered("record_video")) {
474569
const defaultFileName = `simulator_recording_${Date.now()}.mp4`;
475570
const outputFile = ensureAbsolutePath(output_path ?? defaultFileName);
476571

477-
// Build command arguments array
478-
const args = ["simctl", "io", "booted", "recordVideo"];
479-
480-
if (codec) args.push(`--codec=${codec}`);
481-
if (display) args.push(`--display=${display}`);
482-
if (mask) args.push(`--mask=${mask}`);
483-
if (force) args.push("--force");
484-
485-
args.push(outputFile);
486-
487572
// Start the recording process
488-
const recordingProcess = spawn("xcrun", args);
573+
const recordingProcess = spawn("xcrun", [
574+
"simctl",
575+
"io",
576+
"booted",
577+
"recordVideo",
578+
...(codec ? [`--codec=${codec}`] : []),
579+
...(display ? [`--display=${display}`] : []),
580+
...(mask ? [`--mask=${mask}`] : []),
581+
...(force ? ["--force"] : []),
582+
// When passing user-provided values to a command, it's crucial to use `--`
583+
// to separate the command's options from positional arguments.
584+
// This prevents the shell from misinterpreting the arguments as options.
585+
"--",
586+
outputFile,
587+
]);
489588

490589
// Wait for recording to start
491590
await new Promise((resolve, reject) => {
@@ -543,7 +642,7 @@ if (!isToolFiltered("stop_recording")) {
543642
{},
544643
async () => {
545644
try {
546-
await execAsync('pkill -SIGINT -f "simctl.*recordVideo"');
645+
await run("pkill", ["-SIGINT", "-f", "simctl.*recordVideo"]);
547646

548647
// Wait a moment for the video to finalize
549648
await new Promise((resolve) => setTimeout(resolve, 1000));

0 commit comments

Comments
 (0)