Skip to content

Fixed shell command built from environment values #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

odaysec
Copy link

@odaysec odaysec commented Jul 31, 2025

childProcess.exec('echo "" > ' + that.logfile);

childProcess.exec('echo "" > ' + that.logfile);
that.opcode = 'start';

Dynamically constructing a shell command with values from the local environment, such as file paths, may inadvertently change the meaning of the shell command. Such changes can occur when an environment value contains characters that the shell interprets in a special way, for instance quotes and spaces. This can result in the shell command misbehaving, or even allowing a malicious user to execute arbitrary commands on the system.

fix this problem, we should avoid constructing shell commands by concatenating potentially unsafe values and passing them to childProcess.exec. Instead, we should use Node's built-in file system APIs to perform the intended operation. In this case, the command 'echo "" > logfile' is intended to truncate or create an empty log file. The equivalent and safer approach is to use fs.writeFileSync(logfile, '') (for synchronous code) or fs.writeFile(logfile, '', callback) (for asynchronous code). This avoids invoking a shell entirely and is cross-platform.

Specifically, in lib/Local.js, replace both instances of childProcess.exec('echo "" > ' + that.logfile); (in both startSync and start) with the appropriate fs.writeFileSync or fs.writeFile calls. No new imports are needed, as fs is already imported.

@odaysec odaysec requested a review from a team as a code owner July 31, 2025 03:10
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