Skip to content

Commit ba4fee2

Browse files
ar2rsawseenar2rsawseen
authoredNov 27, 2019
Merge pull request #1117 from Countly/cmd_fix
[security] command line fixes
·
v22.09.722.03.6
2 parents 19d4ad0 + 7d3622e commit ba4fee2

File tree

8 files changed

+157
-126
lines changed

8 files changed

+157
-126
lines changed
 

‎api/parts/mgmt/app_users.js

+13-11
Original file line numberDiff line numberDiff line change
@@ -582,10 +582,10 @@ usersApi.deleteExport = function(filename, params, callback) {
582582
}
583583
};
584584

585-
var run_command = function(my_command) {
585+
var run_command = function(my_command, my_args) {
586586
return new Promise(function(resolve, reject) {
587-
var child = spawn(my_command, {
588-
shell: true,
587+
var child = spawn(my_command, my_args, {
588+
shell: false,
589589
cwd: path.resolve(__dirname, './../../../export/AppUser'),
590590
detached: false
591591
}, function(error) {
@@ -615,7 +615,7 @@ var run_command = function(my_command) {
615615
};
616616
var clear_out_empty_files = function(folder) {
617617
return new Promise(function(resolve) {
618-
run_command("find " + folder + " -type f -name '*.json' -size 0 -delete").then(
618+
run_command("find", [folder, "-type", "f", "-name", "*.json", "-size", "0", "-delete"]).then(
619619
function() {
620620
resolve();
621621
},
@@ -727,11 +727,12 @@ usersApi.export = function(app_id, query, params, callback) {
727727
}
728728
var export_filename = 'appUser_' + app_id + '_' + eid;
729729

730-
var dbstr = "";
730+
var dbargs = [];
731731
var export_commands = {};
732732
var db_params = plugins.getDbConnectionParams('countly');
733733
for (var p in db_params) {
734-
dbstr += " --" + p + " " + db_params[p];
734+
dbargs.push("--" + p);
735+
dbargs.push(db_params[p]);
735736
}
736737

737738
plugins.dispatch("/systemlogs", {
@@ -763,16 +764,17 @@ usersApi.export = function(app_id, query, params, callback) {
763764
}
764765
}).then(function() {
765766
//export data from metric_changes
766-
return run_command('mongoexport ' + dbstr + ' --collection metric_changes' + app_id + ' -q \'{uid:{$in: ["' + res[0].uid.join('","') + '"]}}\' --out ' + export_folder + '/metric_changes' + app_id + '.json');
767+
return run_command('mongoexport', [...dbargs, "--collection", "metric_changes" + app_id, "-q", '{uid:{$in: ["' + res[0].uid.join('","') + '"]}}', "--out", export_folder + "/metric_changes" + app_id + ".json"]);
767768
}).then(function() {
768769
//export data from app_users
769-
return run_command('mongoexport ' + dbstr + ' --collection app_users' + app_id + ' -q \'{uid: {$in: ["' + res[0].uid.join('","') + '"]}}\' --out ' + export_folder + '/app_users' + app_id + '.json');
770+
return run_command('mongoexport', [...dbargs, "--collection", "app_users" + app_id, "-q", '{uid:{$in: ["' + res[0].uid.join('","') + '"]}}', "--out", export_folder + "/app_users" + app_id + ".json"]);
770771
}).then(
771772
function() {
772773
//get other export commands from other plugins
773774
plugins.dispatch("/i/app_users/export", {
774775
app_id: app_id,
775-
dbstr: dbstr,
776+
dbstr: "",
777+
dbargs: dbargs,
776778
export_commands: export_commands,
777779
query: query,
778780
uids: res[0].uid,
@@ -781,15 +783,15 @@ usersApi.export = function(app_id, query, params, callback) {
781783
var commands = [];
782784
for (var prop in export_commands) {
783785
for (let k = 0; k < export_commands[prop].length; k++) {
784-
commands.push(run_command(export_commands[prop][k]));
786+
commands.push(run_command(export_commands[prop][k].cmd, export_commands[prop][k].args));
785787
}
786788
}
787789
Promise.all(commands).then(
788790
function() {
789791
//pack export
790792
clear_out_empty_files(path.resolve(__dirname, './../../../export/AppUser/' + export_filename))//remove empty files
791793
.then(function() {
792-
return run_command("tar -zcvf " + export_filename + ".tar.gz" + " " + export_filename);
794+
return run_command("tar", ["-zcvf", export_filename + ".tar.gz", export_filename]);
793795
}) //create archive
794796
.then(function() {
795797
return new Promise(function(resolve, reject) { /*save export in gridFS*/

‎plugins/crashes/api/api.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ plugins.setConfigs("crashes", {
7171
if (!ob.export_commands.crashes) {
7272
ob.export_commands.crashes = [];
7373
}
74-
ob.export_commands.crashes.push('mongoexport ' + ob.dbstr + ' --collection app_crashes' + ob.app_id + ' -q \'{uid:{$in: ["' + uids.join('","') + '"]}}\' --out ' + ob.export_folder + '/crashes' + ob.app_id + '.json');
75-
ob.export_commands.crashes.push('mongoexport ' + ob.dbstr + ' --collection app_crashusers' + ob.app_id + ' -q \'{uid:{$in: ["' + uids.join('","') + '"]}}\' --out ' + ob.export_folder + '/crashusers' + ob.app_id + '.json');
74+
ob.export_commands.crashes.push({cmd: 'mongoexport', args: [...ob.dbargs, '--collection', 'app_crashes' + ob.app_id, '-q', '{uid:{$in: ["' + uids.join('","') + '"]}}', '--out', ob.export_folder + '/crashes' + ob.app_id + '.json']});
75+
ob.export_commands.crashes.push({cmd: 'mongoexport', args: [...ob.dbargs, '--collection', 'app_crashusers' + ob.app_id, '-q', '{uid:{$in: ["' + uids.join('","') + '"]}}', '--out', ob.export_folder + '/crashusers' + ob.app_id + '.json']});
7676
resolve();
7777
}
7878
});

‎plugins/data_migration/api/data_migration_helper.js

+90-74
Large diffs are not rendered by default.

‎plugins/plugin-upload/frontend/app.js

+9-10
Original file line numberDiff line numberDiff line change
@@ -263,20 +263,19 @@ function validate_files(my_path) {
263263

264264
/**Function used to call new child process, separated from parent. For validate_reset()
265265
* @param {string} my_command = command to call
266-
* @param {string} my_dir - folder
266+
* @param {array} my_args - array with command arguments
267267
* @param {string} logpath - path to log file
268268
* @returns {Promise} promise
269269
*/
270-
function run_command(my_command, my_dir, logpath) {
270+
function run_command(my_command, my_args, logpath) {
271271
return new Promise(function(resolve, reject) {
272272
var stdio = ['inherit', 'inherit', 'inherit'];
273273
if (logpath) {
274274
const out = fs.openSync(logpath, 'a');
275275
const err = fs.openSync(logpath, 'a');
276276
stdio = [ 'ignore', out, err ];
277-
278277
}
279-
var child = spawn(my_command, {cwd: __dirname, shell: true, detached: true, stdio: stdio}, function(error) {
278+
var child = spawn(my_command, my_args, {cwd: __dirname, shell: false, detached: true, stdio: stdio}, function(error) {
280279
if (error) {
281280
return reject(Error('error:' + JSON.stringify(error)));
282281
}
@@ -326,8 +325,8 @@ function validate_reset() {
326325
log.d("Attempting disabling plugins, which might cause restart");
327326
tarray = [tstamp];
328327
//try reseting all plugins,enabled in last turn
328+
var pluginlist = [];
329329
if (fs.existsSync(__dirname + '/last_enabled_plugins.json')) {
330-
var pluginlist = [];
331330
let data = fs.readFileSync(__dirname + '/last_enabled_plugins.json');
332331
if (data) {
333332
try {
@@ -342,8 +341,7 @@ function validate_reset() {
342341
var logpath = path.resolve(__dirname, './../../../log/plugins-disable' + (new Date().toISOString().replace('T', ':')) + '.log');
343342

344343
var mydir = path.resolve(__dirname + '/../scripts');
345-
run_command('bash ' + mydir + '/disable_plugins.sh ' + pluginlist.join(' '),
346-
mydir, logpath)
344+
run_command('bash', [mydir + '/disable_plugins.sh', ...pluginlist], logpath)
347345
.then(
348346
function() {
349347
try {
@@ -423,11 +421,12 @@ function extract_files(ext, target_path) {
423421
}
424422
//for other - tar, tar.gz
425423
else {
426-
var command = "tar xzf " + target_path + " -C " + path.resolve(__dirname + '/upload/unpacked');
424+
var command = "tar";
425+
var args = ["xzf", target_path, "-C", path.resolve(__dirname + '/upload/unpacked')];
427426
if (ext === "tar") {
428-
command = "tar xf " + target_path + " -C " + path.resolve(__dirname + '/upload/unpacked');
427+
args = ["xf", target_path, "-C", path.resolve(__dirname + '/upload/unpacked')];
429428
}
430-
run_command(command, null, null).then(function() {
429+
run_command(command, args, null).then(function() {
431430
resolve();
432431
},
433432
function() {

‎plugins/pluginManager.js

+39-25
Original file line numberDiff line numberDiff line change
@@ -806,9 +806,19 @@ var pluginManager = function pluginManager() {
806806
callback = callback || function() {};
807807
var scriptPath = path.join(__dirname, plugin, 'install.js');
808808
var errors = false;
809-
var process = exec("nodejs " + scriptPath, {maxBuffer: 1024 * 20000}, function(error) {
810-
console.log('Done running install.js with %j', error);
811-
if (error) {
809+
var m = cp.spawn("nodejs", [scriptPath]);
810+
811+
m.stdout.on('data', (data) => {
812+
console.log(data.toString());
813+
});
814+
815+
m.stderr.on('data', (data) => {
816+
console.log(data.toString());
817+
});
818+
819+
m.on('close', (code) => {
820+
console.log('Done running install.js with %j', code);
821+
if (parseInt(code, 10) !== 0) {
812822
errors = true;
813823
return callback(errors);
814824
}
@@ -827,14 +837,6 @@ var pluginManager = function pluginManager() {
827837
callback(errors);
828838
});
829839
});
830-
831-
process.stdout.on("data", function(data) {
832-
console.log(data.toString());
833-
});
834-
835-
process.stderr.on("data", function(data) {
836-
console.log(data.toString());
837-
});
838840
};
839841

840842
/**
@@ -848,9 +850,19 @@ var pluginManager = function pluginManager() {
848850
callback = callback || function() {};
849851
var scriptPath = path.join(__dirname, plugin, 'install.js');
850852
var errors = false;
851-
var process = exec("nodejs " + scriptPath, {maxBuffer: 1024 * 20000}, function(error) {
852-
console.log('Done running install.js with %j', error);
853-
if (error) {
853+
var m = cp.spawn("nodejs", [scriptPath]);
854+
855+
m.stdout.on('data', (data) => {
856+
console.log(data.toString());
857+
});
858+
859+
m.stderr.on('data', (data) => {
860+
console.log(data.toString());
861+
});
862+
863+
m.on('close', (code) => {
864+
console.log('Done running install.js with %j', code);
865+
if (parseInt(code, 10) !== 0) {
854866
errors = true;
855867
return callback(errors);
856868
}
@@ -869,14 +881,6 @@ var pluginManager = function pluginManager() {
869881
callback(errors);
870882
});
871883
});
872-
873-
process.stdout.on("data", function(data) {
874-
console.log(data.toString());
875-
});
876-
877-
process.stderr.on("data", function(data) {
878-
console.log(data.toString());
879-
});
880884
};
881885

882886
/**
@@ -890,9 +894,19 @@ var pluginManager = function pluginManager() {
890894
callback = callback || function() {};
891895
var scriptPath = path.join(__dirname, plugin, 'uninstall.js');
892896
var errors = false;
893-
var process = exec("nodejs " + scriptPath, {maxBuffer: 1024 * 20000}, function(error) {
894-
console.log('Done running uninstall.js with %j', error);
895-
if (error) {
897+
var m = cp.spawn("nodejs", [scriptPath]);
898+
899+
m.stdout.on('data', (data) => {
900+
console.log(data.toString());
901+
});
902+
903+
m.stderr.on('data', (data) => {
904+
console.log(data.toString());
905+
});
906+
907+
m.on('close', (code) => {
908+
console.log('Done running uninstall.js with %j', code);
909+
if (parseInt(code, 10) !== 0) {
896910
errors = true;
897911
}
898912
callback(errors);

‎plugins/push/api/api.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -513,10 +513,10 @@ const PUSH_CACHE_GROUP = 'P';
513513
}
514514
});
515515

516-
plugins.register('/i/app_users/export', ({app_id, uids, export_commands, dbstr, export_folder}) => {
516+
plugins.register('/i/app_users/export', ({app_id, uids, export_commands, dbargs, export_folder}) => {
517517
if (uids && uids.length) {
518518
if (!export_commands.push) {
519-
export_commands.push = [`mongoexport ${dbstr} --collection push_${app_id} -q '{uid: {$in: ${JSON.stringify(uids)}}}' --out ${export_folder}/push_${app_id}.json`];
519+
export_commands.push = [{cmd: 'mongoexport', args: [...dbargs, '--collection', `push_${app_id}`, '-q', `{uid: {$in: ${JSON.stringify(uids)}}}`, '--out', `${export_folder}/push_${app_id}.json`]}];
520520
}
521521
}
522522
});

‎plugins/star-rating/api/api.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ const widgetPropertyPreprocessors = {
705705
if (!ob.export_commands.feedback) {
706706
ob.export_commands.feedback = [];
707707
}
708-
ob.export_commands.feedback.push('mongoexport ' + ob.dbstr + ' --collection feedback' + ob.app_id + ' -q \'{uid:{$in: ["' + uids.join('","') + '"]}}\' --out ' + ob.export_folder + '/feedback' + ob.app_id + '.json');
708+
ob.export_commands.feedback.push({cmd: 'mongoexport', args: [...ob.dbargs, '--collection', 'feedback' + ob.app_id, '-q', '{uid:{$in: ["' + uids.join('","') + '"]}}', '--out', ob.export_folder + '/feedback' + ob.app_id + '.json']});
709709
resolve();
710710
}
711711
});

‎plugins/views/api/api.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ const escapedViewSegments = { "name": true, "segment": true, "height": true, "wi
155155
if (!ob.export_commands.views) {
156156
ob.export_commands.views = [];
157157
}
158-
ob.export_commands.views.push('mongoexport ' + ob.dbstr + ' --collection app_userviews' + ob.app_id + ' -q \'{_id:{$in: ["' + uids.join('","') + '"]}}\' --out ' + ob.export_folder + '/app_userviews' + ob.app_id + '.json');
158+
ob.export_commands.views.push({cmd: 'mongoexport', args: [...ob.dbargs, '--collection', 'app_userviews' + ob.app_id, '-q', '{_id:{$in: ["' + uids.join('","') + '"]}}', '--out', ob.export_folder + '/app_userviews' + ob.app_id + '.json']});
159159
resolve();
160160
});
161161
});

0 commit comments

Comments
 (0)
Please sign in to comment.