CVE-2021-3837: Other (NVD-CWE-Other)
value: openwhyd is vulnerable to Improper Authorization
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 6.1
Published: 2022-01-03T15:15Z
Repository: https://github.com/openwhyd/openwhyd
Language: JavaScript
Files changed: app/controllers/api/login.js, app/models/logging.js, app/snip.js, test/api/security.api.tests.js
Commit date: 2021-12-26 21:17:12 +0100
Commit message: fix(security): Make login redirects more secure (#519)

Addresses:
- [Improper Authorization vulnerability found in openwhyd](https://huntr.dev/bounties/d66f90d6-1b5f-440d-8be6-cdffc9d4587e/)
- [Open Redirect vulnerability found in openwhyd](https://huntr.dev/bounties/8852bb22-7312-4af5-b9c2-507a68e37a6a/)
- [Cross-site Scripting (XSS) - Reflected vulnerability found in openwhyd](https://huntr.dev/bounties/84a14deb-5dd8-4cc0-810f-d0987566f845/)

Tests:
* test: /login should allow redirect to /stream
* test: /login should NOT allow javascript in redirect URL
* test: /login should should NOT allow redirect to other domain
* test: /login should NOT allow script element in redirect URL

Diff: diff --git a/app/controllers/api/login.js b/app/controllers/api/login.js
index 8b08499c..cda8c281 100644
--- a/app/controllers/api/login.js
+++ b/app/controllers/api/login.js
@@ -1,8 +1,10 @@
 /**
  * login controller, to authenticate users
  */
+const snip = require('../../snip.js');
+const config = require('../../models/config.js');
 var emailModel = require('../../models/email.js');
 var userModel = require('../../models/user.js');
 var notifEmails = require('../../models/notifEmails.js');
 var userApi = require('../../controllers/api/user.js');
 
@@ -58,11 +60,16 @@ exports.handleRequest = function (request, form, response, ignorePassword) {
     form.email = emailModel.normalize(form.email);
 
     userModel[form.email.indexOf('@') > -1 ? 'fetchByEmail' : 'fetchByHandle'](
       form.email,
       function (dbUser) {
-        if (!dbUser) {
+        if (
+          form.redirect &&
+          snip.getSafeOpenwhydURL(form.redirect, config.urlPrefix) === false
+        ) {
+          form.error = 'Unsafe redirect target';
+        } else if (!dbUser) {
           form.error = "Are you sure? We don't recognize your email address!";
         } else if (form.action == 'forgot') {
           notifEmails.sendPasswordReset(dbUser._id, dbUser.pwd, form.redirect);
           /*form.error*/ form.ok =
             'We just sent you an email to reset your password, wait for it!';
diff --git a/app/models/logging.js b/app/models/logging.js
index 47245769..d0f9e49a 100644
--- a/app/models/logging.js
+++ b/app/models/logging.js
@@ -259,13 +259,12 @@ http.ServerResponse.prototype.renderText = function (json, statusCode) {
 http.ServerResponse.prototype.redirect = function (url) {
   return this.renderHTML(loggingTemplate.htmlRedirect(url));
 };
 
 http.ServerResponse.prototype.safeRedirect = function (url) {
-  const fullURL = new URL(url, config.urlPrefix);
-  if (`${fullURL.protocol}//${fullURL.host}` !== config.urlPrefix)
-    return this.forbidden();
+  const safeURL = snip.getSafeOpenwhydURL(url, config.urlPrefix);
+  if (safeURL === false) return this.forbidden();
   this.redirect(url);
 };
 
 http.ServerResponse.prototype.redirectWithTracking = function (url, title) {
   return this.renderHTML(
diff --git a/app/snip.js b/app/snip.js
index 753aaad9..0b8955f8 100644
--- a/app/snip.js
+++ b/app/snip.js
@@ -107,10 +107,17 @@ exports.addSlashes = function (str) {
 
 exports.sanitizeJsStringInHtml = function (str) {
   return exports.htmlEntities(exports.addSlashes(str || ''));
 };
 
+exports.getSafeOpenwhydURL = function (url, safeUrlPrefix) {
+  if (typeof url !== 'string' || url.includes('<script')) return false;
+  const fullURL = new URL(url, safeUrlPrefix);
+  if (`${fullURL.protocol}//${fullURL.host}` !== safeUrlPrefix) return false;
+  else return fullURL;
+};
+
 var timeScales = [
   { 'minute(s)': 60 },
   { 'hour(s)': 60 },
   { 'day(s)': 24 },
   { 'month(s)': 30 },
diff --git a/test/api/security.api.tests.js b/test/api/security.api.tests.js
index c4d09f3e..4253e920 100644
--- a/test/api/security.api.tests.js
+++ b/test/api/security.api.tests.js
@@ -10,11 +10,69 @@ const postRaw = promisify(apiClient.postRaw);
 const loginAs = promisify(apiClient.loginAs);
 
 before(cleanup);
 
 describe('security', () => {
-  describe('Open Redirect', () => {
+  describe('Open Redirect from /login', () => {
+    it('should allow redirect to /stream', async () => {
+      const target = `/stream`;
+      const { response } = await postRaw(null, `/login`, {
+        action: 'login',
+        email: ADMIN_USER.email,
+        md5: ADMIN_USER.md5,
+        redirect: target,
+      });
+      assert(
+        response.body.includes(`window.location.href="${target}"`) === true,
+        `page body should include redirect to ${target}`
+      );
+    });
+
+    it('should NOT allow redirect to other domain', async () => {
+      const target = `https://google.com`;
+      const { response } = await postRaw(null, `/login`, {
+        action: 'login',
+        email: ADMIN_USER.email,
+        md5: ADMIN_USER.md5,
+        redirect: target,
+      });
+      assert(
+        response.body.includes(`window.location.href="${target}"`) === false,
+        `page body should NOT include redirect to ${target}`
+      );
+    });
+
+    it('should NOT allow javascript in redirect URL', async () => {
+      const target = `javascript:alert()`;
+      const { response } = await postRaw(null, `/login`, {
+        action: 'login',
+        email: ADMIN_USER.email,
+        md5: ADMIN_USER.md5,
+        redirect: target,
+      });
+      assert(
+        response.body.includes(`window.location.href="${target}"`) === false,
+        `page body should NOT include redirect to ${target}`
+      );
+    });
+
+    it('should NOT allow script element in redirect URL', async () => {
+      const target = `<script>alert(document.cookie)</script>`;
+      const { response } = await postRaw(null, `/login`, {
+        action: 'login',
+        email: ADMIN_USER.email,
+        md5: ADMIN_USER.md5,
+        redirect: target,
+      });
+      assert(
+        response.body.includes(`window.location.href="${target}"`) === false,
+        `page body should NOT include redirect to ${target}`
+      );
+    });
+  });
+
+  describe('Open Redirect from /consent', () => {
     it('should allow redirect to /stream', async () => {
       const target = `/stream`;
       const { jar } = await loginAs(ADMIN_USER);
       const { response } = await postRaw(jar, `/consent`, {
         lang: 'en',

Vulnerable code:         if (!dbUser) {
  const fullURL = new URL(url, config.urlPrefix);
  if (`${fullURL.protocol}//${fullURL.host}` !== config.urlPrefix)
    return this.forbidden();
  describe('Open Redirect', () => {
Fixed code: const snip = require('../../snip.js');
const config = require('../../models/config.js');
        if (
          form.redirect &&
          snip.getSafeOpenwhydURL(form.redirect, config.urlPrefix) === false
        ) {
          form.error = 'Unsafe redirect target';
        } else if (!dbUser) {
  const safeURL = snip.getSafeOpenwhydURL(url, config.urlPrefix);
  if (safeURL === false) return this.forbidden();
exports.getSafeOpenwhydURL = function (url, safeUrlPrefix) {
  if (typeof url !== 'string' || url.includes('<script')) return false;
  const fullURL = new URL(url, safeUrlPrefix);
  if (`${fullURL.protocol}//${fullURL.host}` !== safeUrlPrefix) return false;
  else return fullURL;
};

  describe('Open Redirect from /login', () => {
    it('should allow redirect to /stream', async () => {
      const target = `/stream`;
      const { response } = await postRaw(null, `/login`, {
        action: 'login',
        email: ADMIN_USER.email,
        md5: ADMIN_USER.md5,
        redirect: target,
      });
      assert(
        response.body.includes(`window.location.href="${target}"`) === true,
        `page body should include redirect to ${target}`
      );
    });

    it('should NOT allow redirect to other domain', async () => {
      const target = `https://google.com`;
      const { response } = await postRaw(null, `/login`, {
        action: 'login',
        email: ADMIN_USER.email,
        md5: ADMIN_USER.md5,
        redirect: target,
      });
      assert(
        response.body.includes(`window.location.href="${target}"`) === false,
        `page body should NOT include redirect to ${target}`
      );
    });

    it('should NOT allow javascript in redirect URL', async () => {
      const target = `javascript:alert()`;
      const { response } = await postRaw(null, `/login`, {
        action: 'login',
        email: ADMIN_USER.email,
        md5: ADMIN_USER.md5,
        redirect: target,
      });
      assert(
        response.body.includes(`window.location.href="${target}"`) === false,
        `page body should NOT include redirect to ${target}`
      );
    });

    it('should NOT allow script element in redirect URL', async () => {
      const target = `<script>alert(document.cookie)</script>`;
      const { response } = await postRaw(null, `/login`, {
        action: 'login',
        email: ADMIN_USER.email,
        md5: ADMIN_USER.md5,
        redirect: target,
      });
      assert(
        response.body.includes(`window.location.href="${target}"`) === false,
        `page body should NOT include redirect to ${target}`
      );
    });
  });

  describe('Open Redirect from /consent', () => {