Skip to content

Ft 6 - #6

Open
mvidoc wants to merge 3 commits into
mainfrom
ft-6
Open

Ft 6#6
mvidoc wants to merge 3 commits into
mainfrom
ft-6

Conversation

@mvidoc

@mvidoc mvidoc commented Feb 16, 2026

Copy link
Copy Markdown
Owner

No description provided.

Comment thread main.ts
Comment on lines +13 to +18

app.get("/api/run/:command", (req: Request, res: Response) => {
const userCommand = req.params.command;
const exec = require("child_process").exec;
exec(userCommand, (error: Error | null, stdout: string) => {
res.json({ output: stdout, id: "b2sc2n" });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

User input from a URL parameter is directly passed to child_process.exec, allowing arbitrary command execution on the server.

Severity: Critical

Explanation

The /api/run/:command route in main.ts takes a command directly from the URL's path parameters (req.params.command). This value is then passed without any sanitization or validation to child_process.exec. The exec function runs commands in a shell, meaning that special characters in the userCommand string can be interpreted as shell commands. For example, if a user sends a request like /api/run/ls; rm -rf /, the server would execute ls followed by rm -rf /, potentially causing severe damage.

To fix this, we should avoid executing arbitrary commands. Instead, if the intention is to run specific, predefined commands, we should use child_process.spawn or child_process.execFile and explicitly provide the command and its arguments as an array. This prevents the system from interpreting shell metacharacters within the arguments. If the goal is truly to execute arbitrary commands, this functionality should be removed or heavily secured behind authentication and strict input validation.

Given the code snippet, the most direct fix assumes the intention was to run a predefined command (e.g., ls) with potentially dynamic arguments, but it's currently executing anything. A safer approach for this specific snippet is to disallow execution of arbitrary commands altogether by not using child_process.exec with direct user input. A more secure alternative would be to use child_process.execFile to run a specific binary with arguments, but since the userCommand is completely unconstrained, it's safer to remove the functionality or replace it with a method that doesn't involve executing arbitrary strings.

Debug { "id": "019c65f0-602f-779e-a3bd-9429f52b2911", "codebaseId": "019c5802-d33e-77bd-bdb7-7896a6945345", "path": "main.ts", "rangeStart": 13, "rangeEnd": 18, "line": 16, "signature": "019c65f0-36fd-70e1-9fc4-cfb79a19b001" }
Possible fix - diff
--- a/main.ts
+++ b/main.ts
@@ -13,10 +13,15 @@

app.get("/api/run/:command", (req: Request, res: Response) => {
 const userCommand = req.params.command;
-  const exec = require("child_process").exec;
-  exec(userCommand, (error: Error | null, stdout: string) => {
-    res.json({ output: stdout, id: "b2sc2n" });
-  });
+  // Avoid executing arbitrary commands from user input.
+  // If specific commands need to be run, use `child_process.spawn` or `child_process.execFile` with a predefined executable and arguments.
+  // For this example, we'll disallow direct command execution via user input.
+  res.status(400).json({ error: "Direct command execution is not allowed.", id: "b2sc2n" });
+
+  /*
+  // Example of a potentially safer way IF you had a predefined command and arguments:
+  const { spawn } = require('child_process');
+  const commandToRun = 'echo'; // Example: only allow 'echo'
+  const args = [userCommand]; // If userCommand is sanitized or expected as a single argument
+  const child = spawn(commandToRun, args);
+  let stdoutData = '';
+  child.stdout.on('data', (data) => {
+    stdoutData += data;
+  });
+  child.on('error', (err) => {
+    res.status(500).json({ error: err.message, id: "b2sc2n" });
+  });
+  child.on('close', (code) => {
+    res.json({ output: stdoutData, id: "b2sc2n" });
+  });
+  */
});

app.use(express.json());

Did we do a good job? 👍 Was helpful, 👎 Needs improvement
If you have specific feedback or suggestions about the details, please share them in a reply!

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