Fix remote execution bug where email address can contain malicious code

also convert almost all cmd execs for certificates to properly escape arguments

Author: jc21

backend/internal/access-list.js

@@ -1,5 +1,5 @@
 const _                     = require('lodash');
-const fs                    = require('fs');
+const fs                    = require('node:fs');
 const batchflow             = require('batchflow');
 const logger                = require('../logger').access;
 const error                 = require('../lib/error');
@@ -38,7 +38,7 @@ const internalAccessList = {
 			.then((row) => {
 				data.id = row.id;
 
-				let promises = [];
+				const promises = [];
 
 				// Now add the items
 				data.items.map((item) => {
@@ -116,7 +116,7 @@ const internalAccessList = {
 			.then((row) => {
 				if (row.id !== data.id) {
 					// Sanity check that something crazy hasn't happened
-					throw new error.InternalValidationError('Access List could not be updated, IDs do not match: ' + row.id + ' !== ' + data.id);
+					throw new error.InternalValidationError(`Access List could not be updated, IDs do not match: ${row.id} !== ${data.id}`);
 				}
 			})
 			.then(() => {
@@ -135,10 +135,10 @@ const internalAccessList = {
 			.then(() => {
 				// Check for items and add/update/remove them
 				if (typeof data.items !== 'undefined' && data.items) {
-					let promises      = [];
-					let items_to_keep = [];
+					const promises      = [];
+					const items_to_keep = [];
 
-					data.items.map(function (item) {
+					data.items.map((item) => {
 						if (item.password) {
 							promises.push(accessListAuthModel
 								.query()
@@ -154,7 +154,7 @@ const internalAccessList = {
 						}
 					});
 
-					let query = accessListAuthModel
+					const query = accessListAuthModel
 						.query()
 						.delete()
 						.where('access_list_id', data.id);
@@ -175,9 +175,9 @@ const internalAccessList = {
 			.then(() => {
 				// Check for clients and add/update/remove them
 				if (typeof data.clients !== 'undefined' && data.clients) {
-					let promises = [];
+					const promises = [];
 
-					data.clients.map(function (client) {
+					data.clients.map((client) => {
 						if (client.address) {
 							promises.push(accessListClientModel
 								.query()
@@ -190,7 +190,7 @@ const internalAccessList = {
 						}
 					});
 
-					let query = accessListClientModel
+					const query = accessListClientModel
 						.query()
 						.delete()
 						.where('access_list_id', data.id);
@@ -249,7 +249,7 @@ const internalAccessList = {
 
 		return access.can('access_lists:get', data.id)
 			.then((access_data) => {
-				let query = accessListModel
+				const query = accessListModel
 					.query()
 					.select('access_list.*', accessListModel.raw('COUNT(proxy_host.id) as proxy_host_count'))
 					.leftJoin('proxy_host', function() {
@@ -267,7 +267,7 @@ const internalAccessList = {
 				}
 
 				if (typeof data.expand !== 'undefined' && data.expand !== null) {
-					query.withGraphFetched('[' + data.expand.join(', ') + ']');
+					query.withGraphFetched(`[${data.expand.join(', ')}]`);
 				}
 
 				return query.then(utils.omitRow(omissions()));
@@ -327,7 +327,7 @@ const internalAccessList = {
 									// 3. reconfigure those hosts, then reload nginx
 
 									// set the access_list_id to zero for these items
-									row.proxy_hosts.map(function (val, idx) {
+									row.proxy_hosts.map((_val, idx) => {
 										row.proxy_hosts[idx].access_list_id = 0;
 									});
 
@@ -340,11 +340,11 @@ const internalAccessList = {
 					})
 					.then(() => {
 						// delete the htpasswd file
-						let htpasswd_file = internalAccessList.getFilename(row);
+						const htpasswd_file = internalAccessList.getFilename(row);
 
 						try {
 							fs.unlinkSync(htpasswd_file);
-						} catch (err) {
+						} catch (_err) {
 							// do nothing
 						}
 					})
@@ -374,7 +374,7 @@ const internalAccessList = {
 	getAll: (access, expand, search_query) => {
 		return access.can('access_lists:list')
 			.then((access_data) => {
-				let query = accessListModel
+				const query = accessListModel
 					.query()
 					.select('access_list.*', accessListModel.raw('COUNT(proxy_host.id) as proxy_host_count'))
 					.leftJoin('proxy_host', function() {
@@ -393,19 +393,19 @@ const internalAccessList = {
 				// Query is used for searching
 				if (typeof search_query === 'string') {
 					query.where(function () {
-						this.where('name', 'like', '%' + search_query + '%');
+						this.where('name', 'like', `%${search_query}%`);
 					});
 				}
 
 				if (typeof expand !== 'undefined' && expand !== null) {
-					query.withGraphFetched('[' + expand.join(', ') + ']');
+					query.withGraphFetched(`[${expand.join(', ')}]`);
 				}
 
 				return query.then(utils.omitRows(omissions()));
 			})
 			.then((rows) => {
 				if (rows) {
-					rows.map(function (row, idx) {
+					rows.map((row, idx) => {
 						if (typeof row.items !== 'undefined' && row.items) {
 							rows[idx] = internalAccessList.maskItems(row);
 						}
@@ -424,7 +424,7 @@ const internalAccessList = {
 	 * @returns {Promise}
 	 */
 	getCount: (user_id, visibility) => {
-		let query = accessListModel
+		const query = accessListModel
 			.query()
 			.count('id as count')
 			.where('is_deleted', 0);
@@ -445,7 +445,7 @@ const internalAccessList = {
 	 */
 	maskItems: (list) => {
 		if (list && typeof list.items !== 'undefined') {
-			list.items.map(function (val, idx) {
+			list.items.map((val, idx) => {
 				let repeat_for = 8;
 				let first_char = '*';
 
@@ -468,7 +468,7 @@ const internalAccessList = {
 	 * @returns {String}
 	 */
 	getFilename: (list) => {
-		return '/data/access/' + list.id;
+		return `/data/access/${list.id}`;
 	},
 
 	/**
@@ -479,15 +479,15 @@ const internalAccessList = {
 	 * @returns {Promise}
 	 */
 	build: (list) => {
-		logger.info('Building Access file #' + list.id + ' for: ' + list.name);
+		logger.info(`Building Access file #${list.id} for: ${list.name}`);
 
 		return new Promise((resolve, reject) => {
-			let htpasswd_file = internalAccessList.getFilename(list);
+			const htpasswd_file = internalAccessList.getFilename(list);
 
 			// 1. remove any existing access file
 			try {
 				fs.unlinkSync(htpasswd_file);
-			} catch (err) {
+			} catch (_err) {
 				// do nothing
 			}
 
@@ -504,14 +504,14 @@ const internalAccessList = {
 				if (list.items.length) {
 					return new Promise((resolve, reject) => {
 						batchflow(list.items).sequential()
-							.each((i, item, next) => {
+							.each((_i, item, next) => {
 								if (typeof item.password !== 'undefined' && item.password.length) {
-									logger.info('Adding: ' + item.username);
+									logger.info(`Adding: ${item.username}`);
 
 									utils.execFile('openssl', ['passwd', '-apr1', item.password])
 										.then((res) => {
 											try {
-												fs.appendFileSync(htpasswd_file, item.username + ':' + res + '\n', {encoding: 'utf8'});
+												fs.appendFileSync(htpasswd_file, `${item.username}:${res}\n`, {encoding: 'utf8'});
 											} catch (err) {
 												reject(err);
 											}
@@ -528,7 +528,7 @@ const internalAccessList = {
 								reject(err);
 							})
 							.end((results) => {
-								logger.success('Built Access file #' + list.id + ' for: ' + list.name);
+								logger.success(`Built Access file #${list.id} for: ${list.name}`);
 								resolve(results);
 							});
 					});