Skip to content

Commit 26b9dfe

Browse files
committed
Address data migration review feedback
1 parent c0d261d commit 26b9dfe

4 files changed

Lines changed: 19 additions & 5 deletions

File tree

plugins/data_migration/api/api.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ function trim_ending_slashes(address) {
411411
//delete info file
412412
try {
413413
var importInfoPath = common.resolvePathInBase(path.resolve(__dirname, './../import'), exportid + '.json');
414-
if (importInfoPath) {
414+
if (importInfoPath && fs.existsSync(importInfoPath)) {
415415
fs.unlinkSync(importInfoPath);
416416
}
417417
}

plugins/data_migration/api/data_migration_helper.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,11 @@ module.exports = function(my_db) {
220220
return new Promise(function(resolve, reject) {
221221
my_exportid = safeDataMigrationId(my_exportid);
222222
if (my_exportid) {
223-
var baseFolder = path.resolve(__dirname, './../' + folder);
224223
if (['import', 'export'].indexOf(folder) === -1) {
225-
reject(Error('data-migration.invalid-exportid'));
224+
reject(Error('data-migration.invalid-folder'));
226225
return;
227226
}
227+
var baseFolder = path.resolve(__dirname, './../' + folder);
228228
if (remove_archive) {
229229
var archivePath = common.resolvePathInBase(baseFolder, my_exportid + '.tar.gz');
230230
if (archivePath && fs.existsSync(archivePath)) {

plugins/data_migration/frontend/app.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,12 @@ function safeFilename(filename) {
3232
if (!err && data) {
3333
var myfile = common.resolvePathInBase(path.resolve(__dirname, './../export'), exportid + '.tar.gz');
3434
if (data.export_path && data.export_path !== '') {
35-
myfile = data.export_path;
35+
var customExportPath = path.resolve(data.export_path + "");
36+
if (path.basename(customExportPath) !== exportid + '.tar.gz') {
37+
res.status(400).send('Invalid export file');
38+
return;
39+
}
40+
myfile = customExportPath;
3641
}
3742
if (fs.existsSync(myfile)) {
3843
res.set('Content-Type', 'application/x-gzip');
@@ -44,6 +49,14 @@ function safeFilename(filename) {
4449
return;
4550
}
4651
}
52+
else if (err) {
53+
res.status(500).send('Error loading export file');
54+
return;
55+
}
56+
else {
57+
res.status(404).send('Export file not found');
58+
return;
59+
}
4760
});
4861
}
4962
if (req.query && req.query.logfile) {

plugins/data_migration/tests/index.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ var path = require("path");
77
var fs = require("fs"),
88
readline = require('readline'),
99
stream = require('stream');
10+
var os = require('os');
1011
var cp = require('child_process'); //call process
1112
const exec = cp.exec; //for calling command line
1213
const fse = require('fs-extra'); // delete folders
@@ -853,7 +854,7 @@ describe("Testing data migration plugin", function() {
853854
});
854855
});
855856
it("rejects path traversal in import delete exportid", function(done) {
856-
var traversalTarget = "/tmp/countly_data_migration_path_traversal_test";
857+
var traversalTarget = path.join(os.tmpdir(), "countly_data_migration_path_traversal_test_" + Date.now());
857858
var traversalExportId = "../../../../../../../../.." + traversalTarget;
858859

859860
fs.writeFileSync(traversalTarget, "test");

0 commit comments

Comments
 (0)