feat(js): add external input in filename (#313)

Bearer · Feb 23, 2024 · fc79b86 · fc79b86
1 parent f2aa5f1
commit fc79b86
Show file tree

Hide file tree

Showing 3 changed files with 200 additions and 0 deletions.
diff --git a/rules/javascript/lang/non_literal_fs_filename.yml b/rules/javascript/lang/non_literal_fs_filename.yml
@@ -0,0 +1,166 @@
+imports:
+  - javascript_shared_import_library
+patterns:
+  - pattern: |
+      $<FS>.$<METHOD>($<...>$<INPUT>$<...>)
+    filters:
+      - variable: FS
+        detection: javascript_lang_non_literal_fs_filename_fs_init
+        scope: cursor
+      - variable: METHOD
+        values:
+          - access
+          - appendFile
+          - chmod
+          - chown
+          - close
+          - copyFile
+          - copyFile
+          - cp
+          - cp
+          - createReadStream
+          - createWriteStream
+          - exists
+          - fchmod
+          - fchown
+          - fdatasync
+          - fstat
+          - fsync
+          - ftruncate
+          - futimes
+          - lchmod
+          - lchown
+          - lutimes
+          - link
+          - link
+          - lstat
+          - mkdir
+          - mkdtemp
+          - open
+          - opendir
+          - read
+          - read
+          - readdir
+          - readFile
+          - readlink
+          - readv
+          - realpath
+          - realpath
+          - rename
+          - rename
+          - rmdir
+          - rm
+          - stat
+          - symlink
+          - symlink
+          - truncate
+          - unlink
+          - unwatchFile
+          - utimes
+          - watch
+          - watchFile
+          - write
+          - writeFile
+          - writev
+          - accessSync
+          - appendFileSync
+          - chmodSync
+          - chownSync
+          - closeSync
+          - copyFileSync
+          - copyFileSync
+          - cpSync
+          - cpSync
+          - existsSync
+          - fchmodSync
+          - fchownSync
+          - fdatasyncSync
+          - fstatSync
+          - fsyncSync
+          - ftruncateSync
+          - futimesSync
+          - lchmodSync
+          - lchownSync
+          - lutimesSync
+          - linkSync
+          - linkSync
+          - lstatSync
+          - mkdirSync
+          - mkdtempSync
+          - opendirSync
+          - openSync
+          - readdirSync
+          - readFileSync
+          - readlinkSync
+          - readSync
+          - readSync
+          - readvSync
+          - realpathync
+          - realpathSync
+          - renameSync
+          - renameSync
+          - rmdirSync
+          - rmSync
+          - statSync
+          - symlinkSync
+          - symlinkSync
+          - truncateSync
+          - unlinkSync
+          - utimesSync
+          - writeFileSync
+          - writeSync
+          - writevSync
+      - not:
+          variable: INPUT
+          detection: string_literal
+          scope: result
+auxiliary:
+  - id: javascript_lang_non_literal_fs_filename_fs_init
+    patterns:
+      - pattern: $<LIBRARY>
+        filters:
+          - variable: LIBRARY
+            detection: javascript_shared_import_library
+            scope: cursor
+            filters:
+              - variable: JAVASCRIPT_SHARED_IMPORT_LIBRARY_NAME
+                string_regex: \A(?i)fs(\/promises)?\z
+      - import $<!>$<_> from "fs"
+      - import * as $<!>$<_> from "fs"
+      - import $<!>$<_> from "fs/promises"
+      - import * as $<!>$<_> from "fs/promises"
+languages:
+  - javascript
+severity: warning
+metadata:
+  description: Unsanitized non-literal filename detected
+  remediation_message: |
+    ## Description
+
+    The use of dynamic data with the file system module is bad security practice as it could allow attackers to access unauthorized or hidden files and folders.
+
+    ## Remediations
+
+    ✅ Always ensure that dynamic data and function arguments are sanitized
+
+    ❌ Do not pass dynamic data or function arguments directly to the fs module. Use hard-coded string literals and control logic instead
+
+    ```javascript
+      function write(filename) {
+        switch(filename) {
+          case "hello.txt":
+          fs.writeCreateFile("hello.txt");
+          break;
+
+          case ...
+        }
+      }
+    ```
+
+    ## References
+
+    - [OWASP path traversal](https://owasp.org/www-community/attacks/Path_Traversal)
+  cwe_id:
+    - 73
+  id: javascript_lang_non_literal_fs_filename
+  documentation_url: https://docs.bearer.com/reference/rules/javascript_lang_non_literal_fs_filename
diff --git a/tests/javascript/lang/non_literal_fs_filename/test.js b/tests/javascript/lang/non_literal_fs_filename/test.js
@@ -0,0 +1,18 @@
+const {
+  createNewInvoker,
+  getEnvironment,
+} = require("../../../helper.js")
+const { ruleId, ruleFile, testBase } = getEnvironment(__dirname)
+
+describe(ruleId, () => {
+  const invoke = createNewInvoker(ruleId, ruleFile, testBase)
+
+  test("non_literal_fs_filename", () => {
+    const testCase = "app.js"
+
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+})
diff --git a/tests/javascript/lang/non_literal_fs_filename/testdata/app.js b/tests/javascript/lang/non_literal_fs_filename/testdata/app.js
@@ -0,0 +1,16 @@
+import Fs from 'fs';
+
+export function bad(options) {
+  const { cwd } = options;
+  // bearer:expected javascript_lang_non_literal_fs_filename
+  stdioTarget = Fs.createWriteStream(options.writeLogsToPath, 'utf8');
+
+  // bearer:expected javascript_lang_non_literal_fs_filename
+  if (!Fs.statSync(cwd).isDirectory()) {
+    throw new Error(`cwd "${cwd}" exists but is not a directory`);
+  }
+}
+
+export function ok() {
+  stdioTarget = Fs.createWriteStream('some-string-literal', 'utf8');
+}