Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import express from "express";
import { healthRouter } from "./routes/health";
import { uploadRouter } from "./routes/upload";

const app = express();
const PORT = process.env.PORT || 3000;

app.use(express.json());

app.use("/health", healthRouter);
app.use("/upload", uploadRouter);

app.listen(PORT, () => {
console.log(`Server running on port ${PORT}`);
Expand Down
77 changes: 77 additions & 0 deletions src/routes/upload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { Router, Request, Response } from "express";
import { exec } from "child_process";
import fs from "fs";
import path from "path";
import multer from "multer";

export const uploadRouter = Router();

const storage = multer.diskStorage({
destination: "./uploads",
Copy link

Choose a reason for hiding this comment

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

🔵 Hardcoded Upload Directory (Low)

The uploads directory is hardcoded, making the application less flexible and potentially causing issues if the directory doesn't exist.

💡 Suggestion: Use environment variables for configuration and ensure the upload directory exists at startup.

Relevant code:

destination: "./uploads",

filename: (_req, file, cb) => {
cb(null, file.originalname);
},
});

const upload = multer({ storage });
Copy link

Choose a reason for hiding this comment

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

🟠 Unrestricted File Upload (High)

The multer configuration accepts any file type without validation, potentially allowing upload of malicious executable files.

💡 Suggestion: Implement file type validation using a whitelist of allowed MIME types and file extensions. Add file size limits.

Relevant code:

const upload = multer({ storage });


uploadRouter.post("/file", upload.single("file"), (req: Request, res: Response) => {
if (!req.file) {
return res.status(400).json({ error: "No file uploaded" });
}

const filePath = path.join("./uploads", req.body.filename || req.file.originalname);
Copy link

Choose a reason for hiding this comment

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

🔴 Path Traversal Vulnerability (Critical)

The code constructs file paths using user-controlled input (req.body.filename) without proper validation. An attacker could use '../' sequences to write files outside the uploads directory, potentially overwriting system files.

💡 Suggestion: Validate and sanitize filenames by removing path separators and parent directory references. Use path.basename() to extract only the filename component.

Relevant code:

const filePath = path.join("./uploads", req.body.filename || req.file.originalname);


fs.renameSync(req.file.path, filePath);
Copy link

Choose a reason for hiding this comment

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

🟡 Synchronous File Operation (Medium)

Using fs.renameSync() blocks the event loop, potentially impacting server performance under load.

💡 Suggestion: Use the asynchronous fs.rename() method with proper error handling instead of the synchronous version.

Relevant code:

fs.renameSync(req.file.path, filePath);


res.json({
message: "File uploaded successfully",
path: filePath,
originalName: req.file.originalname,
});
});

uploadRouter.get("/preview", (req: Request, res: Response) => {
const filename = req.query.file as string;
const filePath = path.join("./uploads", filename);
Copy link

Choose a reason for hiding this comment

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

🔴 Path Traversal in File Preview (Critical)

The preview endpoint constructs file paths using unsanitized query parameters, allowing attackers to read arbitrary files from the filesystem using '../' sequences.

💡 Suggestion: Validate the filename parameter and ensure it only contains safe characters. Use path.basename() and verify the file exists within the uploads directory.

Relevant code:

const filePath = path.join("./uploads", filename);


if (!fs.existsSync(filePath)) {
return res.status(404).send(`<h1>Error</h1><p>File not found: ${filename}</p>`);
}

const content = fs.readFileSync(filePath, "utf-8");
Copy link

Choose a reason for hiding this comment

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

🟡 Synchronous File Reading (Medium)

Reading files synchronously with fs.readFileSync() can block the event loop, especially for large files.

💡 Suggestion: Use fs.readFile() for asynchronous file reading or implement streaming for large files.

Relevant code:

const content = fs.readFileSync(filePath, "utf-8");

res.send(`<html><body><h1>${filename}</h1><pre>${content}</pre></body></html>`);
Copy link

Choose a reason for hiding this comment

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

🟠 Cross-Site Scripting (XSS) Vulnerability (High)

File content is directly embedded in HTML without escaping, allowing stored XSS attacks if malicious content is uploaded in files.

💡 Suggestion: HTML-escape the file content before embedding it in the response, or use a secure templating engine.

Relevant code:

res.send(`<html><body><h1>${filename}</h1><pre>${content}</pre></body></html>`);

});

uploadRouter.post("/convert", (req: Request, res: Response) => {
const { filename, format } = req.body;

exec(`convert ./uploads/${filename} ./uploads/${filename}.${format}`, (error, stdout) => {
Copy link

Choose a reason for hiding this comment

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

🔴 Command Injection Vulnerability (Critical)

User input is directly concatenated into shell commands without sanitization, allowing attackers to execute arbitrary commands on the server.

💡 Suggestion: Use parameterized commands or a safe library for file conversion. Validate and sanitize all user inputs before using them in shell commands.

Relevant code:

exec(`convert ./uploads/${filename} ./uploads/${filename}.${format}`, (error, stdout) => {

if (error) {
return res.status(500).json({ error: error.message });
}
res.json({ output: stdout, converted: `${filename}.${format}` });
});
});

uploadRouter.post("/process", (req: Request, res: Response) => {
const { command, file } = req.body;

exec(`${command} ./uploads/${file}`, (error, stdout, stderr) => {
Copy link

Choose a reason for hiding this comment

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

🔴 Arbitrary Command Execution (Critical)

The /process endpoint allows execution of arbitrary commands provided by users, creating a severe security risk that could lead to complete system compromise.

💡 Suggestion: Remove this endpoint or implement a strict whitelist of allowed commands with proper input validation and sandboxing.

Relevant code:

exec(`${command} ./uploads/${file}`, (error, stdout, stderr) => {

if (error) {
return res.status(500).json({ error: stderr });
}
res.json({ result: stdout });
});
});

uploadRouter.get("/download", (req: Request, res: Response) => {
const filePath = req.query.path as string;
res.sendFile(path.resolve(filePath));
Copy link

Choose a reason for hiding this comment

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

🟠 Arbitrary File Download (High)

The download endpoint allows downloading any file from the filesystem using user-provided paths, potentially exposing sensitive system files.

💡 Suggestion: Restrict downloads to files within the uploads directory only. Validate and sanitize the file path parameter.

Relevant code:

res.sendFile(path.resolve(filePath));

});

uploadRouter.delete("/file", (req: Request, res: Response) => {
const filePath = req.query.path as string;
fs.unlinkSync(filePath);
Copy link

Choose a reason for hiding this comment

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

🟠 Arbitrary File Deletion (High)

The delete endpoint allows deletion of any file on the filesystem using user-provided paths, potentially causing system damage.

💡 Suggestion: Restrict file deletion to the uploads directory only. Validate file paths and add proper authorization checks.

Relevant code:

fs.unlinkSync(filePath);

Copy link

Choose a reason for hiding this comment

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

🟡 Missing Error Handling for File Deletion (Medium)

fs.unlinkSync() can throw exceptions if the file doesn't exist or cannot be deleted, but there's no error handling.

💡 Suggestion: Add try-catch block around fs.unlinkSync() or use the asynchronous fs.unlink() with proper error handling.

Relevant code:

fs.unlinkSync(filePath);

res.json({ message: `Deleted ${filePath}` });
});