CVE-2020-7649: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: This affects the package snyk-broker before 4.73.0. It allows arbitrary file reads for users with access to Snyk's internal network via directory traversal.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 4.9
Published: 2022-07-25T14:15Z
Repository: https://github.com/snyk/broker
Language: JavaScript
Files changed: lib/filters/index.js, test/fixtures/accept/github.json, test/unit/filters.test.js
Commit date: 2020-03-18 18:21:55 +0000
Commit message: feat: reject paths with directory traversal

Diff: diff --git a/lib/filters/index.js b/lib/filters/index.js
index 1e27249..8cb4918 100644
--- a/lib/filters/index.js
+++ b/lib/filters/index.js
@@ -1,8 +1,9 @@
 const minimatch = require('minimatch');
 const pathRegexp = require('path-to-regexp');
 const qs = require('qs');
+const path = require('path');
 const undefsafe = require('undefsafe');
 const replace = require('../replace-vars');
 const authHeader = require('../auth-header');
 const tryJSONParse = require('../try-json-parse');
 const logger = require('../log');
@@ -30,11 +31,11 @@ module.exports = ruleSource => {
   logger.info({ rulesCount: rules.length }, 'loading new rules');
 
   // array of entries with
   const tests = rules.map(entry => {
     const keys = [];
-    let { method, origin, path, valid, stream } = entry;
+    let { method, origin, path: entryPath, valid, stream } = entry;
     method = (method || 'get').toLowerCase();
     valid = valid || [];
 
     const bodyFilters = valid.filter(v => !!v.path && !v.regex);
     const bodyRegexFilters = valid.filter(v => !!v.path && !!v.regex);
@@ -42,31 +43,36 @@ module.exports = ruleSource => {
 
     // now track if there's any values that we need to interpolate later
     const fromConfig = {};
 
     // slightly bespoke version of replace-vars.js
-    path = (path || '').replace(/(\${.*?})/g, (_, match) => {
+    entryPath = (entryPath || '').replace(/(\${.*?})/g, (_, match) => {
       const key = match.slice(2, -1); // ditch the wrappers
       fromConfig[key] = config[key] || '';
       return ':' + key;
     });
 
     origin = replace(origin, config);
 
-    if (path[0] !== '/') {
-      path = '/' + path;
+    if (entryPath[0] !== '/') {
+      entryPath = '/' + entryPath;
     }
 
-    logger.info({ method, path }, 'adding new filter rule');
-    const regexp = pathRegexp(path, keys);
+    logger.info({ method, path: entryPath }, 'adding new filter rule');
+    const regexp = pathRegexp(entryPath, keys);
 
     return (req) => {
       // check the request method
       if (req.method.toLowerCase() !== method && method !== 'any') {
         return false;
       }
 
+      // Do not allow directory traversal
+      if (path.normalize(req.url) !== req.url) {
+        return false;
+      }
+
       // Discard any fragments before further processing
       const mainURI = req.url.split('#')[0];
 
       // query params might contain additional "?"s, only split on the 1st one
       const parts = mainURI.split('?');
@@ -131,11 +137,11 @@ module.exports = ruleSource => {
         if (!isValid) {
           return false;
         }
       }
 
-      logger.debug({ path, origin, url, querystring }, 'rule matched');
+      logger.debug({ path: entryPath, origin, url, querystring }, 'rule matched');
 
       querystring = (querystring) ? `?${querystring}` : '';
       return {
         url: origin + url + querystring,
         auth: entry.auth && authHeader(entry.auth),
diff --git a/test/fixtures/accept/github.json b/test/fixtures/accept/github.json
index 0d28074..82a8907 100644
--- a/test/fixtures/accept/github.json
+++ b/test/fixtures/accept/github.json
@@ -6,8 +6,14 @@
     {
       "//": "used to determine the full dependency tree",
       "method": "GET",
       "path": "/repos/:name/:repo/contents/:path*/package.json",
       "origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
+    },
+    {
+      "//": "used to whitelist a folder",
+      "method": "GET",
+      "path": "/repos/:name/:repo/contents/:path*/docs/*",
+      "origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
     } 
   ]
 }
diff --git a/test/unit/filters.test.js b/test/unit/filters.test.js
index 00131c8..6e24605 100644
--- a/test/unit/filters.test.js
+++ b/test/unit/filters.test.js
@@ -5,11 +5,11 @@ const Filters = require('../../lib/filters');
 
 const jsonBuffer = (body) => Buffer.from(JSON.stringify(body));
 
 test('Filter on URL', t => {
   t.test('for GitHub private filters', (t) => {
-    t.plan(3);
+    t.plan(4);
     
     const ruleSource = require(__dirname + '/../fixtures/accept/github.json');
     const filter = Filters(ruleSource.private);
     t.pass('Filters loaded');
 
@@ -37,10 +37,22 @@ test('Filter on URL', t => {
       });
   
       t.end();
     });
 
+    t.test('should block when path includes directory traversal', (t) => {
+      filter({
+        url: '/repos/angular/angular/contents/path/to/docs/../../sensitive/file.js',
+        method: 'GET',
+      }, (error, res) => {
+        t.equal(error.message, 'blocked', 'has been blocked');
+        t.equal(res, undefined, 'no follow allowed');
+      });
+
+      t.end();
+    });
+
     t.end();
   });
 
   t.end();
 });

Vulnerable code:     let { method, origin, path, valid, stream } = entry;
    path = (path || '').replace(/(\${.*?})/g, (_, match) => {
    if (path[0] !== '/') {
      path = '/' + path;
    logger.info({ method, path }, 'adding new filter rule');
    const regexp = pathRegexp(path, keys);
      logger.debug({ path, origin, url, querystring }, 'rule matched');
    t.plan(3);
Fixed code: const path = require('path');
    let { method, origin, path: entryPath, valid, stream } = entry;
    entryPath = (entryPath || '').replace(/(\${.*?})/g, (_, match) => {
    if (entryPath[0] !== '/') {
      entryPath = '/' + entryPath;
    logger.info({ method, path: entryPath }, 'adding new filter rule');
    const regexp = pathRegexp(entryPath, keys);
      // Do not allow directory traversal
      if (path.normalize(req.url) !== req.url) {
        return false;
      }

      logger.debug({ path: entryPath, origin, url, querystring }, 'rule matched');
    },
    {
      "//": "used to whitelist a folder",
      "method": "GET",
      "path": "/repos/:name/:repo/contents/:path*/docs/*",
      "origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
    t.plan(4);
    t.test('should block when path includes directory traversal', (t) => {
      filter({
        url: '/repos/angular/angular/contents/path/to/docs/../../sensitive/file.js',
        method: 'GET',
      }, (error, res) => {
        t.equal(error.message, 'blocked', 'has been blocked');
        t.equal(res, undefined, 'no follow allowed');
      });

      t.end();
    });