Merge pull request #3093 from erik-krogh/MorePathSinks

Approved by asgerf
2026-04-23 07:45:17 +02:00 · 2020-03-24 14:26:41 +00:00
parent fe00d1cbf4 36981f385a
commit ce0b72f949
6 changed files with 235 additions and 5 deletions
--- a/javascript/ql/src/semmle/javascript/Promises.qll
+++ b/javascript/ql/src/semmle/javascript/Promises.qll
@@ -167,6 +167,7 @@ module PromiseTypeTracking {
   * Gets the result from a single step through a promise, from `pred` to `result` summarized by `summary`.
   * This can be loading a resolved value from a promise, storing a value in a promise, or copying a resolved value from one promise to another.
   */
+  pragma[inline]
  DataFlow::SourceNode promiseStep(DataFlow::SourceNode pred, StepSummary summary) {
    exists(PromiseFlowStep step, string field | field = Promises::valueProp() |
      summary = LoadStep(field) and
--- a/javascript/ql/src/semmle/javascript/frameworks/Files.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Files.qll
@@ -201,7 +201,18 @@ private class RecursiveReadDir extends FileSystemAccess, FileNameProducer, DataF

  override DataFlow::Node getAPathArgument() { result = getArgument(0) }

-  override DataFlow::Node getAFileName() { result = getCallback([1 .. 2]).getParameter(1) }
+  override DataFlow::Node getAFileName() { result = trackFileSource(DataFlow::TypeTracker::end()) }
+
+  private DataFlow::SourceNode trackFileSource(DataFlow::TypeTracker t) {
+    t.start() and result = getCallback([1 .. 2]).getParameter(1)
+    or
+    t.startInPromise() and not exists(getCallback([1 .. 2])) and result = this
+    or
+    // Tracking out of a promise
+    exists(DataFlow::TypeTracker t2 |
+      result = PromiseTypeTracking::promiseStep(trackFileSource(t2), t, t2)
+    )
+  }
 }

 /**
@@ -220,10 +231,24 @@ private module JSONFile {

    override DataFlow::Node getAPathArgument() { result = getArgument(0) }

-    override DataFlow::Node getADataNode() {
-      this.getCalleeName() = "readFile" and result = getCallback([1 .. 2]).getParameter(1)
+    override DataFlow::Node getADataNode() { result = trackRead(DataFlow::TypeTracker::end()) }
+
+    private DataFlow::SourceNode trackRead(DataFlow::TypeTracker t) {
+      this.getCalleeName() = "readFile" and
+      (
+        t.start() and result = getCallback([1 .. 2]).getParameter(1)
+        or
+        t.startInPromise() and not exists(getCallback([1 .. 2])) and result = this
+      )
      or
-      this.getCalleeName() = "readFileSync" and result = this
+      t.start() and
+      this.getCalleeName() = "readFileSync" and
+      result = this
+      or
+      // Tracking out of a promise
+      exists(DataFlow::TypeTracker t2 |
+        result = PromiseTypeTracking::promiseStep(trackRead(t2), t, t2)
+      )
    }
  }

@@ -243,6 +268,122 @@ private module JSONFile {
  }
 }

+/**
+ * A call to the library `load-json-file`.
+ */
+private class LoadJsonFile extends FileSystemReadAccess, DataFlow::CallNode {
+  LoadJsonFile() {
+    this = DataFlow::moduleImport("load-json-file").getACall()
+    or
+    this = DataFlow::moduleMember("load-json-file", "sync").getACall()
+  }
+
+  override DataFlow::Node getAPathArgument() { result = getArgument(0) }
+
+  override DataFlow::Node getADataNode() { result = trackRead(DataFlow::TypeTracker::end()) }
+
+  private DataFlow::SourceNode trackRead(DataFlow::TypeTracker t) {
+    this.getCalleeName() = "sync" and t.start() and result = this
+    or
+    not this.getCalleeName() = "sync" and t.startInPromise() and result = this
+    or
+    // Tracking out of a promise
+    exists(DataFlow::TypeTracker t2 |
+      result = PromiseTypeTracking::promiseStep(trackRead(t2), t, t2)
+    )
+  }
+}
+
+/**
+ * A call to the library `write-json-file`.
+ */
+private class WriteJsonFile extends FileSystemWriteAccess, DataFlow::CallNode {
+  WriteJsonFile() {
+    this = DataFlow::moduleImport("write-json-file").getACall()
+    or
+    this = DataFlow::moduleMember("write-json-file", "sync").getACall()
+  }
+
+  override DataFlow::Node getAPathArgument() { result = getArgument(0) }
+
+  override DataFlow::Node getADataNode() { result = getArgument(1) }
+}
+
+/**
+ * A call to the library `walkdir`.
+ */
+private class WalkDir extends FileNameProducer, FileSystemAccess, DataFlow::CallNode {
+  WalkDir() {
+    this = DataFlow::moduleImport("walkdir").getACall()
+    or
+    this = DataFlow::moduleMember("walkdir", "sync").getACall()
+    or
+    this = DataFlow::moduleMember("walkdir", "async").getACall()
+  }
+
+  override DataFlow::Node getAPathArgument() { result = getArgument(0) }
+
+  override DataFlow::Node getAFileName() { result = trackFileSource(DataFlow::TypeTracker::end()) }
+
+  private DataFlow::SourceNode trackFileSource(DataFlow::TypeTracker t) {
+    not this.getCalleeName() = any(string s | s = "sync" or s = "async") and
+    t.start() and
+    (
+      result = getCallback(getNumArgument() - 1).getParameter(0)
+      or
+      result = getAMethodCall(EventEmitter::on()).getCallback(1).getParameter(0)
+    )
+    or
+    t.start() and this.getCalleeName() = "sync" and result = this
+    or
+    t.startInPromise() and this.getCalleeName() = "async" and result = this
+    or
+    // Tracking out of a promise
+    exists(DataFlow::TypeTracker t2 |
+      result = PromiseTypeTracking::promiseStep(trackFileSource(t2), t, t2)
+    )
+  }
+}
+
+/**
+ * A call to the library `globule`.
+ */
+private class Globule extends FileNameProducer, FileSystemAccess, DataFlow::CallNode {
+  Globule() {
+    this = DataFlow::moduleMember("globule", "find").getACall()
+    or
+    this = DataFlow::moduleMember("globule", "match").getACall()
+    or
+    this = DataFlow::moduleMember("globule", "isMatch").getACall()
+    or
+    this = DataFlow::moduleMember("globule", "mapping").getACall()
+    or
+    this = DataFlow::moduleMember("globule", "findMapping").getACall()
+  }
+
+  override DataFlow::Node getAPathArgument() {
+    (this.getCalleeName() = "match" or this.getCalleeName() = "isMatch") and
+    result = getArgument(1)
+    or
+    this.getCalleeName() = "mapping" and
+    (
+      result = getAnArgument() and not exists(result.getALocalSource().getAPropertyWrite("src"))
+      or
+      result = getAnArgument().getALocalSource().getAPropertyWrite("src").getRhs()
+    )
+  }
+
+  override DataFlow::Node getAFileName() {
+    result = this and
+    (
+      this.getCalleeName() = "find" or
+      this.getCalleeName() = "match" or
+      this.getCalleeName() = "findMapping" or
+      this.getCalleeName() = "mapping"
+    )
+  }
+}
+
 /**
 * A file system access made by a NodeJS library.
 * This class models multiple NodeJS libraries that access files.
@@ -257,6 +398,10 @@ private class LibraryAccess extends FileSystemAccess, DataFlow::InvokeNode {
      or
      this = DataFlow::moduleImport("rimraf").getACall()
      or
+      this = DataFlow::moduleImport("readdirp").getACall()
+      or
+      this = DataFlow::moduleImport("walker").getACall()
+      or
      this =
        DataFlow::moduleMember("node-dir",
          any(string s |
--- a/javascript/ql/test/library-tests/frameworks/Concepts/FileAccess.expected
+++ b/javascript/ql/test/library-tests/frameworks/Concepts/FileAccess.expected
@@ -16,11 +16,34 @@ getPathArgument
 | file-access.js:47:1:47:36 | vfs.des ... true }) | file-access.js:47:10:47:13 | './' |
 | file-access.js:51:1:51:36 | ncp("fr ... rr) {}) | file-access.js:51:5:51:10 | "from" |
 | file-access.js:51:1:51:36 | ncp("fr ... rr) {}) | file-access.js:51:13:51:16 | "to" |
+| file-access.js:56:23:56:46 | loadJso ... .json') | file-access.js:56:36:56:45 | 'foo.json' |
+| file-access.js:57:14:57:42 | loadJso ... .json') | file-access.js:57:32:57:41 | 'foo.json' |
+| file-access.js:62:5:62:42 | writeJs ...  true}) | file-access.js:62:19:62:28 | 'bar.json' |
+| file-access.js:63:2:63:60 | writeJs ... : " "}) | file-access.js:63:21:63:30 | 'bar.json' |
+| file-access.js:67:1:67:35 | readdir ... *.js'}) | file-access.js:67:10:67:12 | '.' |
+| file-access.js:70:1:72:2 | recursi ... es);\\n}) | file-access.js:70:11:70:29 | "directory/to/read" |
+| file-access.js:73:1:73:30 | recursi ... /read") | file-access.js:73:11:73:29 | "directory/to/read" |
+| file-access.js:75:1:75:29 | jsonfil ... .json') | file-access.js:75:19:75:28 | 'baz.json' |
+| file-access.js:79:2:81:3 | walk('. ... h);\\n\\t}) | file-access.js:79:7:79:11 | '../' |
+| file-access.js:82:16:82:26 | walk('../') | file-access.js:82:21:82:25 | '../' |
+| file-access.js:84:2:86:3 | walk.sy ... h);\\n\\t}) | file-access.js:84:12:84:16 | '../' |
+| file-access.js:87:14:87:29 | walk.sync('../') | file-access.js:87:24:87:28 | '../' |
+| file-access.js:88:21:88:37 | walk.async('../') | file-access.js:88:32:88:36 | '../' |
+| file-access.js:92:1:92:15 | walker('/etc/') | file-access.js:92:8:92:14 | '/etc/' |
+| tst-file-names.js:43:15:43:50 | globule ... o.js"]) | tst-file-names.js:43:40:43:49 | ["foo.js"] |
+| tst-file-names.js:44:12:44:49 | globule ... o.js"]) | tst-file-names.js:44:39:44:48 | ["foo.js"] |
+| tst-file-names.js:46:12:46:51 | globule ... .js"]}) | tst-file-names.js:46:34:46:49 | ["a.js", "b.js"] |
+| tst-file-names.js:47:12:47:52 | globule ... b.js"]) | tst-file-names.js:47:28:47:51 | ["foo/a ... /b.js"] |
 getReadNode
 | file-access.js:25:1:25:59 | jsonfil ... bj) {}) | file-access.js:25:52:25:54 | obj |
 | file-access.js:26:1:26:39 | jsonfil ... .json') | file-access.js:26:1:26:39 | jsonfil ... .json') |
+| file-access.js:56:23:56:46 | loadJso ... .json') | file-access.js:56:17:56:46 | await l ... .json') |
+| file-access.js:57:14:57:42 | loadJso ... .json') | file-access.js:57:14:57:42 | loadJso ... .json') |
+| file-access.js:75:1:75:29 | jsonfil ... .json') | file-access.js:75:36:75:38 | obj |
 getWriteNode
 | file-access.js:15:1:15:60 | writeFi ... rr) {}) | file-access.js:15:31:15:36 | 'Data' |
 | file-access.js:18:1:18:59 | writeFi ... tions]) | file-access.js:18:37:18:47 | "More data" |
 | file-access.js:28:1:28:60 | jsonfil ... rr) {}) | file-access.js:28:38:28:40 | obj |
 | file-access.js:29:1:29:45 | jsonfil ... ', obj) | file-access.js:29:42:29:44 | obj |
+| file-access.js:62:5:62:42 | writeJs ...  true}) | file-access.js:62:31:62:41 | {bar: true} |
+| file-access.js:63:2:63:60 | writeJs ... : " "}) | file-access.js:63:33:63:44 | {bar: false} |
--- a/javascript/ql/test/library-tests/frameworks/Concepts/FileNameSource.expected
+++ b/javascript/ql/test/library-tests/frameworks/Concepts/FileNameSource.expected
@@ -1,4 +1,11 @@
 | file-access.js:22:39:22:43 | files |
+| file-access.js:70:47:70:51 | files |
+| file-access.js:73:37:73:42 | files2 |
+| file-access.js:79:23:79:26 | path |
+| file-access.js:83:30:83:37 | filename |
+| file-access.js:84:2:86:3 | walk.sy ... h);\\n\\t}) |
+| file-access.js:87:14:87:29 | walk.sync('../') |
+| file-access.js:88:15:88:37 | await w ... ('../') |
 | tst-file-names.js:7:1:7:10 | walkSync() |
 | tst-file-names.js:9:35:9:44 | stats.name |
 | tst-file-names.js:11:1:11:12 | glob.sync(_) |
@@ -15,3 +22,8 @@
 | tst-file-names.js:34:15:34:29 | await globby(_) |
 | tst-file-names.js:36:16:36:38 | await f ... sync(_) |
 | tst-file-names.js:38:16:38:57 | await f ...  => {}) |
+| tst-file-names.js:42:17:42:39 | globule ... /*.js') |
+| tst-file-names.js:43:15:43:50 | globule ... o.js"]) |
+| tst-file-names.js:45:12:45:42 | globule ... /*.js") |
+| tst-file-names.js:46:12:46:51 | globule ... .js"]}) |
+| tst-file-names.js:47:12:47:52 | globule ... b.js"]) |
--- a/javascript/ql/test/library-tests/frameworks/Concepts/file-access.js
+++ b/javascript/ql/test/library-tests/frameworks/Concepts/file-access.js
@@ -48,4 +48,45 @@ vfs.dest('./', { sourcemaps: true });


 var ncp = require('ncp').ncp;
-ncp("from", "to", function (err) {});
+ncp("from", "to", function (err) {});
+
+
+const loadJsonFile = require('load-json-file');
+(async () => {
+    console.log(await loadJsonFile('foo.json'));
+	console.log(loadJsonFile.sync('foo.json'));
+})();
+
+const writeJsonFile = require('write-json-file');
+ (async () => {
+    writeJsonFile('bar.json', {bar: true});
+	writeJsonFile.sync('bar.json', {bar: false}, {indent: " "})
+})();
+
+var readdirp = require("readdirp");
+readdirp('.', {fileFilter: '*.js'}).on('data', (entry) => { /* stream and promise api not modelled yet */ })
+
+var recursive = require("recursive-readdir");
+recursive("directory/to/read", function (err, files) {
+  console.log(files);
+});
+recursive("directory/to/read").then(files2 => console.log(files2));
+
+jsonfile.readFile('baz.json').then(obj => console.log(obj))
+
+(async function () {
+	var walk = require('walkdir');
+	walk('../', function(path, stat) {
+	  console.log('found: ', path);
+	});
+	var emitter = walk('../');
+	emitter.on('file', function(filename, stat) { });
+	walk.sync('../', function(path, stat) {
+	  console.log('found sync:', path);
+	});
+	var paths = walk.sync('../');
+	let result = await walk.async('../')
+})();
+
+var walker = require("walker");
+walker('/etc/').filterDir(() => {}).on('entry', () => {}); // only file access modelled.
--- a/javascript/ql/test/library-tests/frameworks/Concepts/tst-file-names.js
+++ b/javascript/ql/test/library-tests/frameworks/Concepts/tst-file-names.js
@@ -37,3 +37,11 @@ async function foo() {

  var files2 = await fastGlob.async(_).catch((wat) => {});
 }
+
+var globule = require('globule');
+var filepaths = globule.find('**/*.js');
+var matches = globule.match('**/*.js', ["foo.js"])
+var bool = globule.isMatch('**/*.js', ["foo.js"])
+var map1 = globule.findMapping("foo/*.js")
+var map2 = globule.mapping({src: ["a.js", "b.js"]})
+var map3 = globule.mapping(["foo/a.js", "foo/b.js"])