CVE-2022-0144: Improper Privilege Management (CWE-269)
value: shelljs is vulnerable to Improper Privilege Management
CWE description: The product does not properly assign, modify, track, or check privileges for an actor, creating an unintended sphere of control for that actor.
CVSS v3 base score: 7.1
Published: 2022-01-11T07:15Z
Repository: https://github.com/shelljs/shelljs
Language: JavaScript
Files changed: src/exec.js
Commit date: 2022-01-06 21:18:18 -0800
Commit message: fix(exec): lockdown file permissions (#1060)

This locks down file permissions used by the internal implementation of
`shell.exec()`.

Issue #1058
Tested manually using the documented scenarios
Diff: diff --git a/src/exec.js b/src/exec.js
index de3322c..78faf13 100644
--- a/src/exec.js
+++ b/src/exec.js
@@ -46,11 +46,28 @@ function execSync(cmd, opts, pipe) {
     pipe: pipe,
     stdoutFile: stdoutFile,
     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'),
     paramsFile,
   ];
@@ -89,10 +106,11 @@ function execSync(cmd, opts, pipe) {
     stdout = fs.readFileSync(stdoutFile, opts.encoding);
     stderr = fs.readFileSync(stderrFile, opts.encoding);
   }
 
   // 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) {}
 
   if (code !== 0) {

Vulnerable code:   fs.writeFileSync(paramsFile, JSON.stringify(paramsToSerialize), 'utf8');
Fixed code:   // 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));
  // and we locked down permissions (see the note above).