From b5d3a10dc27bd0851322752a1afd039e82f45b2d Mon Sep 17 00:00:00 2001
From: ansuz <ansuz@transitiontech.ca>
Date: Fri, 9 Oct 2020 13:28:13 +0530
Subject: [PATCH] guard against possible typeErrors from unvalidated config
 input

---
 lib/commands/admin-rpc.js | 10 +++++-----
 lib/historyKeeper.js      | 14 ++++++++++----
 server.js                 | 17 +++++++++--------
 3 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/lib/commands/admin-rpc.js b/lib/commands/admin-rpc.js
index 5604ec923..d178f39db 100644
--- a/lib/commands/admin-rpc.js
+++ b/lib/commands/admin-rpc.js
@@ -176,13 +176,12 @@ var restoreArchivedDocument = function (Env, Server, cb) {
 };
 
 // CryptPad_AsyncStore.rpc.send('ADMIN', [ 'ADMIN_DECREE', ['RESTRICT_REGISTRATION', [true]]], console.log)
-var adminDecree = function (Env, Server, cb, data, safeKey) {
+var adminDecree = function (Env, Server, cb, data, unsafeKey) {
     var value = data[1];
     if (!Array.isArray(value)) { return void cb('INVALID_DECREE'); }
 
     var command = value[0];
     var args = value[1];
-    var unsafeKey = Util.unescapeKeyCharacters(safeKey);
 
 /*
 
@@ -253,15 +252,16 @@ Admin.command = function (Env, safeKey, data, _cb, Server) {
     var cb = Util.once(Util.mkAsync(_cb));
 
     var admins = Env.admins;
-    //var unsafeKey = Util.unescapeKeyCharacters(safeKey);
-    if (admins.indexOf(safeKey) === -1) {
+
+    var unsafeKey = Util.unescapeKeyCharacters(safeKey);
+    if (admins.indexOf(unsafeKey) === -1) {
         return void cb("FORBIDDEN");
     }
 
     var command = commands[data[0]];
 
     if (typeof(command) === 'function') {
-        return void command(Env, Server, cb, data, safeKey);
+        return void command(Env, Server, cb, data, unsafeKey);
     }
 
     return void cb('UNHANDLED_ADMIN_COMMAND');
diff --git a/lib/historyKeeper.js b/lib/historyKeeper.js
index cc1ff1fef..5278a771c 100644
--- a/lib/historyKeeper.js
+++ b/lib/historyKeeper.js
@@ -85,6 +85,8 @@ module.exports.create = function (config, cb) {
 
     (function () {
         var custom = config.customLimits;
+        if (!custom) { return; }
+
         var stored = Env.customLimits;
 
         Object.keys(custom).forEach(function (k) {
@@ -147,11 +149,15 @@ module.exports.create = function (config, cb) {
         Core.DEFAULT_LIMIT;
 
     try {
+        // XXX this should be the same as is exposed in server.js
+        // /api/config.adminKeys
         Env.admins = (config.adminKeys || []).map(function (k) {
-            k = k.replace(/\/+$/, '');
-            var s = k.split('/');
-            return s[s.length-1];
-        });
+            try {
+                return Keys.canonicalize(k);
+            } catch (err) {
+                return;
+            }
+        }).filter(Boolean);
     } catch (e) {
         console.error("Can't parse admin keys. Please update or fix your config.js file!");
     }
diff --git a/server.js b/server.js
index 2f8dce43b..5a97c971f 100644
--- a/server.js
+++ b/server.js
@@ -90,8 +90,6 @@ config.flushCache = function () {
     config.log.info("UPDATING_FRESH_KEY", FRESH_KEY);
 };
 
-const clone = (x) => (JSON.parse(JSON.stringify(x)));
-
 var setHeaders = (function () {
     // load the default http headers unless the admin has provided their own via the config file
     var headers;
@@ -99,7 +97,7 @@ var setHeaders = (function () {
     var custom = config.httpHeaders;
     // if the admin provided valid http headers then use them
     if (custom && typeof(custom) === 'object' && !Array.isArray(custom)) {
-        headers = clone(custom);
+        headers = Util.clone(custom);
     } else {
         // otherwise use the default
         headers = Default.httpHeaders();
@@ -120,7 +118,7 @@ var setHeaders = (function () {
         headers['Content-Security-Policy'] = Default.contentSecurity(config.httpUnsafeOrigin);
     }
 
-    const padHeaders = clone(headers);
+    const padHeaders = Util.clone(headers);
     if (typeof(config.padContentSecurity) === 'string') {
         padHeaders['Content-Security-Policy'] = config.padContentSecurity;
     } else {
@@ -202,6 +200,7 @@ app.use(/^\/[^\/]*$/, Express.static('customize.dist'));
 var admins = [];
 try {
     admins = (config.adminKeys || []).map(function (k) {
+        // XXX is there any reason not to use Keys.canonicalize ?
         // return each admin's "unsafeKey"
         // this might throw and invalidate all the other admin's keys
         // but we want to get the admin's attention anyway.
@@ -228,12 +227,12 @@ var serveConfig = (function () {
                 allowSubscriptions: (config.allowSubscriptions === true),
                 websocketPath: config.externalWebsocketURL,
                 httpUnsafeOrigin: config.httpUnsafeOrigin,
-                adminEmail: config.adminEmail,
+                adminEmail: config.adminEmail, // XXX mutable
                 adminKeys: admins,
-                inactiveTime: config.inactiveTime,
+                inactiveTime: config.inactiveTime, // XXX mutable
                 supportMailbox: config.supportMailboxPublicKey,
-                maxUploadSize: config.maxUploadSize,
-                premiumUploadSize: config.premiumUploadSize,
+                maxUploadSize: config.maxUploadSize, // XXX mutable
+                premiumUploadSize: config.premiumUploadSize, // XXX mutable
             }, null, '\t'),
             'obj.httpSafeOrigin = ' + (function () {
                 if (config.httpSafeOrigin) { return '"' + config.httpSafeOrigin + '"'; }
@@ -259,6 +258,8 @@ var serveConfig = (function () {
         }
         // generate a lookup key for the cache
         var cacheKey = host + ':' + cacheString();
+
+        // XXX we must be able to clear the cache when updating any mutable key
         // if there's nothing cached for that key...
         if (!configCache[cacheKey]) {
             // generate the response and cache it in memory