Injection and output
Avoid command injection
Prefer library APIs, pass fixed executable arguments without a shell, and allowlist the small set of operations a feature supports.
A shell turns strings into programs. It reads ;, |, &&, $() and backticks as instructions, not as characters. Pass user input through a shell and you hand that input the power to start new commands, with your server’s permissions.
The shape that fails
Here an image tool builds one command string and lets /bin/sh parse it:
const { exec } = require('node:child_process')
// Vulnerable: the filename is interpreted by /bin/sh
exec(`convert uploads/${name} output.png`)
Now upload a file named photo.jpg; rm -rf /srv/uploads. The shell sees two commands separated by ;. The first converts an image. The second deletes the uploads folder. The upload feature worked exactly as written, and that’s the problem.
You can test this safely on your own machine with a harmless payload. Name the file x; echo pwned > /tmp/proof and check whether /tmp/proof appears. With exec, it does.
Pass arguments, not a command line
Prefer a native library when one exists. For images, sharp does the conversion inside Node with no process at all. If you must spawn a process, choose the executable in code and pass the arguments as an array. execFile does not start a shell, so a ; in an argument is just a semicolon:
const { execFile } = require('node:child_process')
// Safe: fixed executable, arguments never parsed by a shell
execFile('convert', ['--', `uploads/${name}`, 'output.png'], (err) => {
if (err) return handleError(err)
})
Run the same test filename now. convert looks for a file literally called uploads/x; echo pwned > /tmp/proof, fails with unable to open image, and /tmp/proof never appears. That error is the result you want.
The -- marks the end of options. Without it, a filename like -delete could be read as a flag by the tool. Combine this with an allowlist of the operations the feature supports: resize, convert to PNG, and nothing else.
Don’t escape by hand
Every few months someone proposes a function that escapes “the dangerous characters” and keeps exec. Don’t. The list changes across shells and platforms, and one missed character is enough. The array form removes shell parsing from the boundary entirely, so there’s nothing to escape.
Watch for the same bug hiding in other places: child_process.spawn with shell: true, a Makefile target that takes a variable, a cron script that reads a filename from the database. Anywhere a string becomes a command line, ask who controls the string.
Try this on your own project: route the conversion through execFile or a library, then upload filenames with spaces, single quotes, ;, |, and a leading dash. The only file that changes on disk should be the output image.
Lesson completed