Fixed issue where pipe calls from rxjs package would been identified as pipe calls on streams

Author: Napalys

javascript/ql/lib/ext/rxjs.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/javascript-all
+      extensible: typeModel
+    data:
+      - ["NonNodeStream", "rxjs", "Fuzzy"]
+      - ["NonNodeStream", "rxjs/operators", "Fuzzy"]

javascript/ql/src/Quality/UnhandledStreamPipe.ql

@@ -19,7 +19,8 @@ class PipeCall extends DataFlow::MethodCallNode {
     this.getMethodName() = "pipe" and
     this.getNumArgument() = [1, 2] and
     not this.getArgument(0).asExpr() instanceof Function and
-    not this.getArgument(0).asExpr() instanceof ObjectExpr
+    not this.getArgument(0).asExpr() instanceof ObjectExpr and
+    not this.getArgument(0).getALocalSource() = getNonNodeJsStreamType()
   }
 
   /** Gets the source stream (receiver of the pipe call). */
@@ -29,6 +30,14 @@ class PipeCall extends DataFlow::MethodCallNode {
   DataFlow::Node getDestinationStream() { result = this.getArgument(0) }
 }
 
+/**
+ * Gets a reference to a value that is known to not be a Node.js stream.
+ * This is used to exclude pipe calls on non-stream objects from analysis.
+ */
+DataFlow::Node getNonNodeJsStreamType() {
+  result = ModelOutput::getATypeNode("NonNodeStream").asSource()
+}
+
 /**
  * Gets the method names used to register event handlers on Node.js streams.
  * These methods are used to attach handlers for events like `error`.
@@ -181,9 +190,18 @@ predicate hasErrorHandlerRegistered(PipeCall pipeCall) {
   )
 }
 
+/**
+ * Holds if the source or destination of the given pipe call is identified as a non-Node.js stream.
+ */
+predicate hasNonNodeJsStreamSource(PipeCall pipeCall) {
+  streamRef(pipeCall) = getNonNodeJsStreamType() or
+  pipeResultRef(pipeCall) = getNonNodeJsStreamType()
+}
+
 from PipeCall pipeCall
 where
   not hasErrorHandlerRegistered(pipeCall) and
-  not isPipeFollowedByNonStreamAccess(pipeCall)
+  not isPipeFollowedByNonStreamAccess(pipeCall) and
+  not hasNonNodeJsStreamSource(pipeCall)
 select pipeCall,
   "Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped."

javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/rxjsStreams.js

@@ -5,6 +5,6 @@ const { of, from } = rx;
 const { map, filter } = ops;
 
 function f(){
-  of(1, 2, 3).pipe(map(x => x * 2)); // $SPURIOUS:Alert
-  someNonStream().pipe(map(x => x * 2)); // $SPURIOUS:Alert
+  of(1, 2, 3).pipe(map(x => x * 2));
+  someNonStream().pipe(map(x => x * 2));
 }

javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.expected

@@ -1,5 +1,3 @@
-| rxjsStreams.js:8:3:8:35 | of(1, 2 ... x * 2)) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
-| rxjsStreams.js:9:3:9:39 | someNon ... x * 2)) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:4:5:4:28 | stream. ... nation) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:19:5:19:17 | s2.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:45:5:45:30 | stream2 ... ation2) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |