Skip to content

Commit

Permalink
fix(exec): lockdown file permissions (#1060)
Browse files Browse the repository at this point in the history
This locks down file permissions used by the internal implementation of
`shell.exec()`.

Issue #1058
Tested manually using the documented scenarios
  • Loading branch information
nfischer committed Jan 7, 2022
1 parent fcf1651 commit d919d22
Showing 1 changed file with 19 additions and 1 deletion.
20 changes: 19 additions & 1 deletion src/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,24 @@ function execSync(cmd, opts, pipe) {
stderrFile: stderrFile,
};

fs.writeFileSync(paramsFile, JSON.stringify(paramsToSerialize), 'utf8');
// Create the files and ensure these are locked down (for read and write) to
// the current user. The main concerns here are:
//
// * If we execute a command which prints sensitive output, then
// stdoutFile/stderrFile must not be readable by other users.
// * paramsFile must not be readable by other users, or else they can read it
// to figure out the path for stdoutFile/stderrFile and create these first
// (locked down to their own access), which will crash exec() when it tries
// to write to the files.
function writeFileLockedDown(filePath, data) {
fs.writeFileSync(filePath, data, {
encoding: 'utf8',
mode: parseInt('600', 8),
});
}
writeFileLockedDown(stdoutFile, '');
writeFileLockedDown(stderrFile, '');
writeFileLockedDown(paramsFile, JSON.stringify(paramsToSerialize));

var execArgs = [
path.join(__dirname, 'exec-child.js'),
Expand Down Expand Up @@ -91,6 +108,7 @@ function execSync(cmd, opts, pipe) {
}

// No biggie if we can't erase the files now -- they're in a temp dir anyway
// and we locked down permissions (see the note above).
try { common.unlinkSync(paramsFile); } catch (e) {}
try { common.unlinkSync(stderrFile); } catch (e) {}
try { common.unlinkSync(stdoutFile); } catch (e) {}
Expand Down

0 comments on commit d919d22

Please sign in to comment.