Просмотр исходного кода

impl safeRedirect with whitelist mechanism

Yuki Takei 6 лет назад
Родитель
Сommit
db919ee130
2 измененных файлов с 156 добавлено и 19 удалено
  1. 48 19
      src/server/middleware/safe-redirect.js
  2. 108 0
      src/test/middleware/safe-redirect.test.js

+ 48 - 19
src/server/middleware/safe-redirect.js

@@ -2,34 +2,63 @@ const loggerFactory = require('@alias/logger');
 
 const logger = loggerFactory('growi:middleware:safe-redirect');
 
+
+/**
+ * Check whether the redirect url host is in specified whitelist
+ * @param {Array<string>} whitelistOfHosts
+ * @param {string} redirectToFqdn
+ */
+function isInWhitelist(whitelistOfHosts, redirectToFqdn) {
+  if (whitelistOfHosts == null || whitelistOfHosts.length === 0) {
+    return false;
+  }
+
+  const redirectUrl = new URL(redirectToFqdn);
+  return whitelistOfHosts.includes(redirectUrl.hostname) || whitelistOfHosts.includes(redirectUrl.host);
+}
+
 /**
  * Redirect with prevention from Open Redirect
  *
  * Usage: app.use(require('middleware/safe-redirect'))
  */
-module.exports = function(req, res, next) {
+module.exports = (whitelistOfHosts) => {
 
-  // extends res object
-  res.safeRedirect = function(redirectTo) {
-    if (redirectTo == null) {
-      return res.redirect('/');
-    }
+  return function(req, res, next) {
 
-    // prevention from open redirect
-    try {
-      const redirectUrl = new URL(redirectTo, `${req.protocol}://${req.get('host')}`);
-      if (redirectUrl.hostname === req.hostname) {
-        return res.redirect(redirectUrl);
+    // extend res object
+    res.safeRedirect = function(redirectTo) {
+      if (redirectTo == null) {
+        return res.redirect('/');
       }
-      logger.warn('Requested redirect URL is invalid, redirect to root page');
-    }
-    catch (err) {
-      logger.warn('Requested redirect URL is invalid, redirect to root page', err);
-    }
 
-    return res.redirect('/');
-  };
+      try {
+        // check inner redirect
+        const redirectUrl = new URL(redirectTo, `${req.protocol}://${req.get('host')}`);
+        if (redirectUrl.hostname === req.hostname) {
+          logger.debug(`Requested redirect URL (${redirectTo}) is local.`);
+          return res.redirect(redirectUrl.href);
+        }
+        logger.debug(`Requested redirect URL (${redirectTo}) is NOT local.`);
 
-  next();
+        // check whitelisted redirect
+        const isWhitelisted = isInWhitelist(whitelistOfHosts, redirectTo);
+        if (isWhitelisted) {
+          logger.debug(`Requested redirect URL (${redirectTo}) is in whitelist.`, `whitelist=${whitelistOfHosts}`);
+          return res.redirect(redirectTo);
+        }
+        logger.debug(`Requested redirect URL (${redirectTo}) is NOT in whitelist.`, `whitelist=${whitelistOfHosts}`);
+      }
+      catch (err) {
+        logger.warn(`Requested redirect URL (${redirectTo}) is invalid.`, err);
+      }
+
+      logger.warn(`Requested redirect URL (${redirectTo}) is UNSAFE, redirecting to root page.`);
+      return res.redirect('/');
+    };
+
+    next();
+
+  };
 
 };

+ 108 - 0
src/test/middleware/safe-redirect.test.js

@@ -0,0 +1,108 @@
+/* eslint-disable arrow-body-style */
+
+describe('safeRedirect', () => {
+  let safeRedirect;
+
+  const whitelistOfHosts = [
+    'white1.example.com:8080',
+    'white2.example.com',
+  ];
+
+  beforeEach(async(done) => {
+    safeRedirect = require('@server/middleware/safe-redirect')(whitelistOfHosts);
+    done();
+  });
+
+  describe('res.safeRedirect', () => {
+    // setup req/res/next
+    const req = {
+      protocol: 'http',
+      hostname: 'example.com',
+      get: jest.fn().mockReturnValue('example.com'),
+    };
+    const res = {
+      redirect: jest.fn().mockReturnValue('redirect'),
+    };
+    const next = jest.fn();
+
+    test('redirects to \'/\' because specified url causes open redirect vulnerability', () => {
+      safeRedirect(req, res, next);
+
+      const result = res.safeRedirect('//evil.example.com');
+
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledWith('host');
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('/');
+      expect(result).toBe('redirect');
+    });
+
+    test('redirects to \'/\' because specified host without port is not in whitelist', () => {
+      safeRedirect(req, res, next);
+
+      const result = res.safeRedirect('http://white1.example.com/path/to/page');
+
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledWith('host');
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('/');
+      expect(result).toBe('redirect');
+    });
+
+    test('redirects to the specified local url', () => {
+      safeRedirect(req, res, next);
+
+      const result = res.safeRedirect('/path/to/page');
+
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledWith('host');
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('http://example.com/path/to/page');
+      expect(result).toBe('redirect');
+    });
+
+    test('redirects to the specified local url (fqdn)', () => {
+      safeRedirect(req, res, next);
+
+      const result = res.safeRedirect('http://example.com/path/to/page');
+
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledWith('host');
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('http://example.com/path/to/page');
+      expect(result).toBe('redirect');
+    });
+
+    test('redirects to the specified whitelisted url (white1.example.com:8080)', () => {
+      safeRedirect(req, res, next);
+
+      const result = res.safeRedirect('http://white1.example.com:8080/path/to/page');
+
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledWith('host');
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('http://white1.example.com:8080/path/to/page');
+      expect(result).toBe('redirect');
+    });
+
+    test('redirects to the specified whitelisted url (white2.example.com:8080)', () => {
+      safeRedirect(req, res, next);
+
+      const result = res.safeRedirect('http://white2.example.com:8080/path/to/page');
+
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledTimes(1);
+      expect(req.get).toHaveBeenCalledWith('host');
+      expect(res.redirect).toHaveBeenCalledTimes(1);
+      expect(res.redirect).toHaveBeenCalledWith('http://white2.example.com:8080/path/to/page');
+      expect(result).toBe('redirect');
+    });
+
+  });
+
+});