fix(security): Prevent open redirect (#496)

Report: https://www.huntr.dev/bounties/cfc18877-c2ad-41ac-96c7-128a06c51ea9/

* reproduce open redirect problem

* fix: implement and call safe redirect from /consent route

app/controllers/consent.js

@@ -112,7 +112,7 @@ exports.controller = async function (request, getParams, response) {
       r.html = mainTemplate.renderWhydPage(r);
     }
     // call the adequate renderer
-    if (r.redirect) response.redirect(r.redirect);
+    if (r.redirect) response.safeRedirect(r.redirect);
     else if (r.html) response.renderHTML(r.html);
     else response.renderJSON(r);
     // and track visit to that page

app/models/logging.js

@@ -255,10 +255,17 @@ http.ServerResponse.prototype.renderText = function (json, statusCode) {
   );
 };
 
+// TODO: this function is overrided by Express => delete it to prevent ambiguity
 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.toString().startsWith(config.urlPrefix)) return this.forbidden();
+  this.redirect(url);
+};
+
 http.ServerResponse.prototype.redirectWithTracking = function (url, title) {
   return this.renderHTML(
     loggingTemplate.renderRedirectPageWithTracking(url, title)

test/api-client.js

@@ -87,6 +87,13 @@ exports.get = function (jar, url, callback) {
   });
 };
 
+exports.postRaw = function (jar, url, body, callback) {
+  request.post(
+    { jar, url: `${URL_PREFIX}${url}`, body, json: typeof body === 'object' },
+    (error, response, body) => callback(error, { response, body })
+  );
+};
+
 // USER
 
 /*

test/api/security.api.tests.js

@@ -0,0 +1,47 @@
+/* global describe, it, before */
+
+const { promisify } = require('util');
+const assert = require('assert');
+
+const { ADMIN_USER, cleanup, URL_PREFIX } = require('../fixtures.js');
+const apiClient = require('../api-client.js');
+
+const postRaw = promisify(apiClient.postRaw);
+const loginAs = promisify(apiClient.loginAs);
+
+before(cleanup);
+
+describe('security', () => {
+  describe('Open Redirect', () => {
+    it('should allow redirect to /stream', async () => {
+      const target = `/stream`;
+      const { jar } = await loginAs(ADMIN_USER);
+      const { response } = await postRaw(jar, `/consent`, {
+        lang: 'en',
+        redirect: target,
+      });
+      assert.equal(response.statusCode, 302);
+      assert.equal(response.headers.location, target);
+    });
+
+    it(`should allow redirect to ${URL_PREFIX}/stream`, async () => {
+      const target = `${URL_PREFIX}/stream`;
+      const { jar } = await loginAs(ADMIN_USER);
+      const { response } = await postRaw(jar, `/consent`, {
+        lang: 'en',
+        redirect: target,
+      });
+      assert.equal(response.statusCode, 302);
+      assert.equal(response.headers.location, target);
+    });
+
+    it('should NOT allow redirect to other domain', async () => {
+      const { jar } = await loginAs(ADMIN_USER);
+      const { response } = await postRaw(jar, `/consent`, {
+        lang: 'en',
+        redirect: `http://google.com`,
+      });
+      assert.equal(response.statusCode, 403); // forbidden
+    });
+  });
+});