support pino logging calls on request objects

Author: erik-krogh

javascript/ql/src/semmle/javascript/frameworks/Logging.qll

@@ -343,22 +343,43 @@ class StripAnsiStep extends TaintTracking::SharedTaintStep {
  */
 private module Pino {
   /**
-   * Gets a logger instance from the `pino` library.
+   * Gets a logger instance created by importing the `pino` library.
    */
-  private API::Node pino() {
+  private API::Node pinoApi() {
     result = API::moduleImport("pino").getReturn()
     or
-    result = pino().getMember("child").getReturn()
+    result = pinoApi().getMember("child").getReturn()
+  }
+
+  /**
+   * Gets a logger instance from the `pino` library.
+   */
+  private DataFlow::SourceNode pino() {
+    result = pinoApi().getAnImmediateUse()
+    or
+    // `pino` is installed as the "log" property on the request object in `Express` and similar libraries.
+    // in `Hapi` the property is "logger".
+    exists(HTTP::RequestExpr req |
+      result = req.flow().getALocalSource().getAPropertyRead(["log", "logger"])
+    )
+  }
+
+  /**
+   * Gets a reference to a logger method from the `pino` library.
+   */
+  private DataFlow::SourceNode pinoCallee(DataFlow::TypeTracker t) {
+    t.startInProp(["trace", "debug", "info", "warn", "error", "fatal"]) and
+    result = pino()
+    or
+    exists(DataFlow::TypeTracker t2 | result = pinoCallee(t2).track(t2, t))
   }
 
   /**
    * A logging call to the `pino` library.
    */
   private class PinoCall extends LoggerCall {
-    PinoCall() {
-      this = pino().getMember(["trace", "debug", "info", "warn", "error", "fatal"]).getACall()
-    }
+    PinoCall() { this = pinoCallee(DataFlow::TypeTracker::end()).getACall() }
 
-    override DataFlow::Node getAMessageComponent() { result = getArgument(0) }
+    override DataFlow::Node getAMessageComponent() { result = getAnArgument() }
   }
 }

javascript/ql/test/query-tests/Security/CWE-117/LogInjection.expected

@@ -75,6 +75,14 @@ nodes
 | logInjectionBad.js:65:20:65:35 | q.query.username |
 | logInjectionBad.js:67:15:67:22 | username |
 | logInjectionBad.js:67:15:67:22 | username |
+| logInjectionBad.js:74:30:74:37 | username |
+| logInjectionBad.js:74:30:74:37 | username |
+| logInjectionBad.js:83:26:83:33 | username |
+| logInjectionBad.js:83:26:83:33 | username |
+| logInjectionBad.js:91:26:91:33 | username |
+| logInjectionBad.js:91:26:91:33 | username |
+| logInjectionBad.js:105:37:105:44 | username |
+| logInjectionBad.js:105:37:105:44 | username |
 edges
 | logInjectionBad.js:19:9:19:36 | q | logInjectionBad.js:20:20:20:20 | q |
 | logInjectionBad.js:19:13:19:36 | url.par ... , true) | logInjectionBad.js:19:9:19:36 | q |
@@ -146,6 +154,14 @@ edges
 | logInjectionBad.js:64:23:64:29 | req.url | logInjectionBad.js:64:13:64:36 | url.par ... , true) |
 | logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:67:15:67:22 | username |
 | logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:67:15:67:22 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:74:30:74:37 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:74:30:74:37 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:83:26:83:33 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:83:26:83:33 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:91:26:91:33 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:91:26:91:33 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:105:37:105:44 | username |
+| logInjectionBad.js:65:9:65:35 | username | logInjectionBad.js:105:37:105:44 | username |
 | logInjectionBad.js:65:20:65:20 | q | logInjectionBad.js:65:20:65:26 | q.query |
 | logInjectionBad.js:65:20:65:26 | q.query | logInjectionBad.js:65:20:65:35 | q.query.username |
 | logInjectionBad.js:65:20:65:35 | q.query.username | logInjectionBad.js:65:9:65:35 | username |
@@ -166,3 +182,7 @@ edges
 | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:57:17:57:48 | chalk.u ... ername) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
 | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) | logInjectionBad.js:46:23:46:29 | req.url | logInjectionBad.js:58:17:58:59 | stripAn ... rname)) | $@ flows to log entry. | logInjectionBad.js:46:23:46:29 | req.url | User-provided value |
 | logInjectionBad.js:67:15:67:22 | username | logInjectionBad.js:64:23:64:29 | req.url | logInjectionBad.js:67:15:67:22 | username | $@ flows to log entry. | logInjectionBad.js:64:23:64:29 | req.url | User-provided value |
+| logInjectionBad.js:74:30:74:37 | username | logInjectionBad.js:64:23:64:29 | req.url | logInjectionBad.js:74:30:74:37 | username | $@ flows to log entry. | logInjectionBad.js:64:23:64:29 | req.url | User-provided value |
+| logInjectionBad.js:83:26:83:33 | username | logInjectionBad.js:64:23:64:29 | req.url | logInjectionBad.js:83:26:83:33 | username | $@ flows to log entry. | logInjectionBad.js:64:23:64:29 | req.url | User-provided value |
+| logInjectionBad.js:91:26:91:33 | username | logInjectionBad.js:64:23:64:29 | req.url | logInjectionBad.js:91:26:91:33 | username | $@ flows to log entry. | logInjectionBad.js:64:23:64:29 | req.url | User-provided value |
+| logInjectionBad.js:105:37:105:44 | username | logInjectionBad.js:64:23:64:29 | req.url | logInjectionBad.js:105:37:105:44 | username | $@ flows to log entry. | logInjectionBad.js:64:23:64:29 | req.url | User-provided value |

javascript/ql/test/query-tests/Security/CWE-117/logInjectionBad.js

@@ -65,4 +65,47 @@ const server3 = http.createServer((req, res) => {
     let username = q.query.username;
 
     pino.info(username); // NOT OK
+
+    function fastify() {
+        const fastify = require('fastify')({
+            logger: true
+        });
+        fastify.get('/', async (request, reply) => {
+            request.log.info(username); // NOT OK
+            return { hello: 'world' }
+        });
+    }
+
+    function express() {
+        const express = require('express');
+        const app = express();
+        app.get('/', (req, res) => {
+            req.log.info(username); // NOT OK
+            res.send({ hello: 'world' });
+        });
+    }
+
+    function http() {
+        const http = require('http');
+        const server = http.createServer((req, res) => {
+            req.log.info(username); // NOT OK
+            res.end('Hello World\n');
+        });
+        server.listen(3000);
+    }
+
+    function hapi() {
+        const Hapi = require('hapi');
+        const server = new Hapi.Server();
+        server.connection({ port: 3000 });
+        server.route({
+            method: 'GET',
+            path: '/',
+            handler: (request, reply) => {
+                request.logger.info(username); // NOT OK
+                reply({ hello: 'world' });
+            }
+        });
+        server.start();
+    }
 });