From d6167d968c2addd3790ffa9681c5ec697e93ef77 Mon Sep 17 00:00:00 2001 From: Misha Koryak Date: Wed, 16 Apr 2014 23:53:41 -0400 Subject: [PATCH 1/6] - use npmFileMap from npm tarball if present - filter out malicious paths from filemap - clean up some code --- npm-auto-update.js | 89 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 26 deletions(-) diff --git a/npm-auto-update.js b/npm-auto-update.js index 3d688661c..8312ddf86 100644 --- a/npm-auto-update.js +++ b/npm-auto-update.js @@ -28,37 +28,79 @@ var parse = function (json_file, ignore_missing, ignore_parse_fail) { } } +var isValidFileMap = function(pkg){ + var isValidPath = function(p){ + return p !== null && !p.match(/([\//]\.\.[\//])/); //don't allow parent dir access + }; + + if(pkg && pkg.npmFileMap){ + return _.every(pkg.npmFileMap, function(fileSpec){ + if(isValidPath(fileSpec.basePath || "")){ + return _.every(fileSpec.files, isValidPath); + } + return false; + }); + } + return false +}; + var updateLibrary = function (pkg, callback) { + if(!isValidFileMap(pkg)){ + console.log(pkg.npmName+" has a malicious npmFileMap, skipping!"); + return callback(null); + } console.log('Checking versions for ' + pkg.npmName); request.get('http://registry.npmjs.org/' + pkg.npmName, function(result) { _.each(result.body.versions, function(data, version) { - var path = './ajax/libs/' + pkg.name + '/' + version; - console.log(path); - if(!fs.existsSync(path)) { - console.log('dont have', version); - fs.mkdirSync(path); - var url = data.dist.tarball; - var download_file = path + '/dist.tar.gz'; - tarball.extractTarballDownload(url , download_file, path, {}, function(err, result) { - console.log('this is happening sync'); - fs.unlinkSync(download_file); - var folderName = fs.readdirSync(path)[0]; + if(~pkg.name.indexOf("..")){ + console.log(pkg.npmName+" has a malicious package name, skipping! ", pkg.name); + return; + } + var libPath = path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); + var isAllowedPath = function(){ //is path within the lib dir? if not, they shouldnt be writing/reading there + var paths = 1 <= arguments.length ? [].slice.call(arguments, 0) : []; + var re = new RegExp("^"+libPath) + return _.every(paths, function(p) { + p = path.normalize(p); + return p.match(re); + }); + }; + + if(!fs.existsSync(libPath)) { + console.log('dont have', version); + fs.mkdirSync(libPath); + var url = data.dist.tarball; + var downloadFile = libPath + '/dist.tar.gz'; + tarball.extractTarballDownload(url , downloadFile, libPath, {}, function(err, result) { + + fs.unlinkSync(downloadFile); + var folderName = fs.readdirSync(libPath)[0]; + var newPkg = parse(path.join(libPath, folderName, 'package.json')); + if(isValidFileMap(newPkg)){ + pkg.npmFileMap = newPkg.npmFileMap; + } var npmFileMap = pkg.npmFileMap; _.each(npmFileMap, function(fileSpec) { var basePath = fileSpec.basePath || ""; _.each(fileSpec.files, function(file) { - var extractPath = basePath + "/" + file; - var files = glob.sync(path + "/" + folderName + "/" + basePath + "/" + file); + var libContentsPath = path.normalize(path.join(libPath, folderName, basePath, file)); + if(!isAllowedPath(libContentsPath)){ + console.log(pkg.npmName+" contains a malicious file path, skipping: ", libContentsPath); + } + var files = glob.sync(libContentsPath); _.each(files, function(extractFilePath) { - if(extractFilePath.slice(-4) == ".zip") return; - if(extractFilePath.indexOf("dependencies") !== -1) return; - var replacePath = folderName + "/" + basePath + "/"; - replacePath = replacePath.replace(/\/\//g, "/"); + if(extractFilePath.match(/(dependencies|\.zip\s*$)/i)) return; + + var replacePath = path.normalize(path.join(folderName, basePath)); var actualPath = extractFilePath.replace(replacePath, ""); + if(!isAllowedPath(extractFilePath, actualPath)){ + console.log(pkg.npmName+" contains a malicious file path, skipping: ", extractFilePath, actualPath); + return; + } fs.renameSync(extractFilePath, actualPath); }); }); @@ -73,26 +115,21 @@ var updateLibrary = function (pkg, callback) { pkg.version = npmVersion; fs.writeFileSync('ajax/libs/' + pkg.name + '/package.json', JSON.stringify(pkg, null, 2), 'utf8'); - callback(null, pkg['npm-name']); + callback(null); }); } console.log('Looking for npm enabled libraries...'); // load up those files -var packages = glob.sync("./ajax/libs/**/package.json"); +var packages = glob.sync("./ajax/libs/*/package.json"); packages = _(packages).map(function (pkg) { var parsedPkg = parse(pkg); return parsedPkg.npmName ? parsedPkg : null; }).compact().value(); console.log('Found ' + packages.length + ' npm enabled libraries'); -var libraryUpdates = []; -_.each(packages, function(pkg) { - libraryUpdates.push(function (callback) { - updateLibrary(pkg, callback); - });; -}); -async.series(libraryUpdates, function(err, results) { + +async.eachSeries(packages, updateLibrary, function(err) { console.log('Script completed'); }); From c83430923f12f418e96da3f797ae5d98b54bcf3e Mon Sep 17 00:00:00 2001 From: Misha Koryak Date: Thu, 17 Apr 2014 12:50:47 -0400 Subject: [PATCH 2/6] add hipchat notifications for evil npms rebased with latest --- auto-update.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/auto-update.js b/auto-update.js index 7e345864f..c6dbd3530 100644 --- a/auto-update.js +++ b/auto-update.js @@ -66,14 +66,16 @@ var isValidFileMap = function(pkg){ var updateLibrary = function (pkg, callback) { if(!isValidFileMap(pkg)){ - console.log(pkg.npmName+" has a malicious npmFileMap, skipping!"); + console.log(pkg.npmName+" has a malicious npmFileMap"); + hipchat.message('red', pkg.npmName+" has a malicious npmFileMap"); return callback(null); } console.log('Checking versions for ' + pkg.npmName); request.get('http://registry.npmjs.org/' + pkg.npmName, function(result) { _.each(result.body.versions, function(data, version) { if(~pkg.name.indexOf("..")){ - console.log(pkg.npmName+" has a malicious package name, skipping! ", pkg.name); + console.log(pkg.npmName+" has a malicious package name:", pkg.name); + hipchat.message('red', pkg.npmName+" has a malicious package name: "+pkg.name); return; } var libPath = path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); @@ -108,7 +110,8 @@ var updateLibrary = function (pkg, callback) { _.each(fileSpec.files, function(file) { var libContentsPath = path.normalize(path.join(libPath, folderName, basePath, file)); if(!isAllowedPath(libContentsPath)){ - console.log(pkg.npmName+" contains a malicious file path, skipping: ", libContentsPath); + console.log(pkg.npmName+" contains a malicious file path: ", libContentsPath); + hipchat.message('red', pkg.npmName+" contains a malicious file path: "+libContentsPath); } var files = glob.sync(libContentsPath); @@ -118,7 +121,8 @@ var updateLibrary = function (pkg, callback) { var replacePath = path.normalize(path.join(folderName, basePath)); var actualPath = extractFilePath.replace(replacePath, ""); if(!isAllowedPath(extractFilePath, actualPath)){ - console.log(pkg.npmName+" contains a malicious file path, skipping: ", extractFilePath, actualPath); + console.log(pkg.npmName+" contains a malicious file path: ", extractFilePath, actualPath); + hipchat.message('red', pkg.npmName+" contains a malicious file path: "+extractFilePath+' or '+actualPath); return; } fs.renameSync(extractFilePath, actualPath); @@ -146,7 +150,7 @@ console.log('Looking for npm enabled libraries...'); var packages = glob.sync("./ajax/libs/*/package.json"); packages = _(packages).map(function (pkg) { var parsedPkg = parse(pkg); - return parsedPkg.npmName ? parsedPkg : null; + return (parsedPkg.npmName && parsedPkg.npmFileMap) ? parsedPkg : null; }).compact().value(); hipchat.message('green', 'Found ' + packages.length + ' npm enabled libraries'); console.log('Found ' + packages.length + ' npm enabled libraries'); From 4fbed1ede92d6c2b6d2bab07644808ccf7f7c7cd Mon Sep 17 00:00:00 2001 From: Misha Koryak Date: Thu, 17 Apr 2014 18:24:24 -0400 Subject: [PATCH 3/6] refactor code to be more testable --- auto-update.js | 162 +++++++++++++++++++++++---------------- test/auto-update-test.js | 8 ++ 2 files changed, 103 insertions(+), 67 deletions(-) create mode 100644 test/auto-update-test.js diff --git a/auto-update.js b/auto-update.js index c6dbd3530..4eb5c1513 100644 --- a/auto-update.js +++ b/auto-update.js @@ -64,6 +64,92 @@ var isValidFileMap = function(pkg){ return false }; +var error = function(msg, name){ + var err = new Error(msg); + err.name = name; + console.log(msg); + hipchat.message('red', msg); + return err; +} +error.PKG_NAME = 'BadPackageName' +error.FILE_PATH = 'BadFilePath' + + +var isAllowedPathFn = function(libPath){ //is path within the lib dir? if not, they shouldnt be writing/reading there + return function(){ + var paths = 1 <= arguments.length ? [].slice.call(arguments, 0) : []; + var re = new RegExp("^"+libPath) + return _.every(paths, function(p) { + p = path.normalize(p); + return p.match(re); + }); + } +}; + +var processNewVersion = function(pkg, libPath, folderName){ + var isAllowedPath = isAllowedPathFn(libPath); + + var newPkg = parse(path.join(libPath, folderName, 'package.json')); + if(isValidFileMap(newPkg)){ + pkg.npmFileMap = newPkg.npmFileMap; + } + var npmFileMap = pkg.npmFileMap; + var errors = []; + + _.each(npmFileMap, function(fileSpec) { + var basePath = fileSpec.basePath || ""; + + _.each(fileSpec.files, function(file) { + var libContentsPath = path.normalize(path.join(libPath, folderName, basePath, file)); + if(!isAllowedPath(libContentsPath)){ + errors.push(error(pkg.npmName+" contains a malicious file path: "+libContentsPath, error.FILE_PATH)); + return + } + var files = glob.sync(libContentsPath); + + _.each(files, function(extractFilePath) { + if(extractFilePath.match(/(dependencies|\.zip\s*$)/i)) return; + + var replacePath = path.normalize(path.join(folderName, basePath)); + var actualPath = extractFilePath.replace(replacePath, ""); + if(!isAllowedPath(extractFilePath, actualPath)){ + errors.push(error(pkg.npmName+" contains a malicious file path: "+extractFilePath+' or '+actualPath, error.FILE_PATH)); + return; + } + fs.renameSync(extractFilePath, actualPath); + }); + }); + }); + return errors; +} + +var updateLibraryVersion = function(pkg, tarballUrl, version, cb) { + if(~pkg.name.indexOf("..")){ + return cb(error(pkg.npmName+" has a malicious package name:"+ pkg.name, error.PKG_NAME)); + } + var libPath = path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); + + + if(!fs.existsSync(libPath)) { + fs.mkdirSync(libPath); + var url = tarballUrl; + var downloadFile = libPath + '/dist.tar.gz'; + tarball.extractTarballDownload(url , downloadFile, libPath, {}, function(err, result) { + + fs.unlinkSync(downloadFile); + var folderName = fs.readdirSync(libPath)[0]; + processNewVersion(pkg, libPath, folderName); + + fs.removeSync(path.join(libPath, folderName)); + + newVersionCount++; + console.log("Do not have version", version, "of", pkg.npmName); + cb() + }); + } else { + cb() + } +}; var updateLibrary = function (pkg, callback) { if(!isValidFileMap(pkg)){ console.log(pkg.npmName+" has a malicious npmFileMap"); @@ -72,75 +158,17 @@ var updateLibrary = function (pkg, callback) { } console.log('Checking versions for ' + pkg.npmName); request.get('http://registry.npmjs.org/' + pkg.npmName, function(result) { - _.each(result.body.versions, function(data, version) { - if(~pkg.name.indexOf("..")){ - console.log(pkg.npmName+" has a malicious package name:", pkg.name); - hipchat.message('red', pkg.npmName+" has a malicious package name: "+pkg.name); - return; - } - var libPath = path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); + async.eachLimit(_.pairs(result.body.versions), 5, function(p, cb){ //extract 5 at a time + var data = p[1]; + var version = p[0]; + updateLibraryVersion(pkg, data.dist.tarball, version, cb) + }, function(err){ + var npmVersion = result.body['dist-tags'] && result.body['dist-tags'].latest || 0; + pkg.version = npmVersion; + fs.writeFileSync('ajax/libs/' + pkg.name + '/package.json', JSON.stringify(pkg, null, 2), 'utf8'); - var isAllowedPath = function(){ //is path within the lib dir? if not, they shouldnt be writing/reading there - var paths = 1 <= arguments.length ? [].slice.call(arguments, 0) : []; - var re = new RegExp("^"+libPath) - return _.every(paths, function(p) { - p = path.normalize(p); - return p.match(re); - }); - }; - - if(!fs.existsSync(libPath)) { - console.log('dont have', version); - fs.mkdirSync(libPath); - var url = data.dist.tarball; - var downloadFile = libPath + '/dist.tar.gz'; - tarball.extractTarballDownload(url , downloadFile, libPath, {}, function(err, result) { - - fs.unlinkSync(downloadFile); - var folderName = fs.readdirSync(libPath)[0]; - var newPkg = parse(path.join(libPath, folderName, 'package.json')); - if(isValidFileMap(newPkg)){ - pkg.npmFileMap = newPkg.npmFileMap; - } - var npmFileMap = pkg.npmFileMap; - - _.each(npmFileMap, function(fileSpec) { - var basePath = fileSpec.basePath || ""; - - _.each(fileSpec.files, function(file) { - var libContentsPath = path.normalize(path.join(libPath, folderName, basePath, file)); - if(!isAllowedPath(libContentsPath)){ - console.log(pkg.npmName+" contains a malicious file path: ", libContentsPath); - hipchat.message('red', pkg.npmName+" contains a malicious file path: "+libContentsPath); - } - var files = glob.sync(libContentsPath); - - _.each(files, function(extractFilePath) { - if(extractFilePath.match(/(dependencies|\.zip\s*$)/i)) return; - - var replacePath = path.normalize(path.join(folderName, basePath)); - var actualPath = extractFilePath.replace(replacePath, ""); - if(!isAllowedPath(extractFilePath, actualPath)){ - console.log(pkg.npmName+" contains a malicious file path: ", extractFilePath, actualPath); - hipchat.message('red', pkg.npmName+" contains a malicious file path: "+extractFilePath+' or '+actualPath); - return; - } - fs.renameSync(extractFilePath, actualPath); - }); - }); - }); - - fs.removeSync(path + '/' + folderName); - }); - newVersionCount++; - console.log("Do not have version", version, "of", pkg.npmName); - } + callback(null); }); - var npmVersion = result.body['dist-tags'] && result.body['dist-tags'].latest || 0; - pkg.version = npmVersion; - fs.writeFileSync('ajax/libs/' + pkg.name + '/package.json', JSON.stringify(pkg, null, 2), 'utf8'); - - callback(null); }); } diff --git a/test/auto-update-test.js b/test/auto-update-test.js new file mode 100644 index 000000000..38eb8ce94 --- /dev/null +++ b/test/auto-update-test.js @@ -0,0 +1,8 @@ +var assert = require("assert"), + path = require("path"), + fs = require("fs"), + glob = require("glob"), + vows = require("vows-si"), + jsv = require("JSV").JSV.createEnvironment(); + +//TODO \ No newline at end of file From 58d83e358a276ce464e1ee8b3b6bd1e720ee88ca Mon Sep 17 00:00:00 2001 From: Misha Koryak Date: Fri, 18 Apr 2014 17:24:59 -0400 Subject: [PATCH 4/6] more cleanup for tests --- auto-update.js | 127 ++++++++++++++++++++++++++++++++++--------------- auto-update.sh | 2 +- 2 files changed, 90 insertions(+), 39 deletions(-) diff --git a/auto-update.js b/auto-update.js index 4eb5c1513..77b31fb09 100644 --- a/auto-update.js +++ b/auto-update.js @@ -1,4 +1,12 @@ -var Hipchat = require('node-hipchat'); +var Hipchat = require('node-hipchat'), + path = require("path"), + fs = require("fs-extra"), + glob = require("glob"), + _ = require('lodash'), + request = require("superagent"), + async = require("async"), + tarball = require('tarball-extract'), + mkdirp = require('mkdirp'); var HC = new Hipchat(process.env.HIPCHAT); var hipchat = { @@ -17,14 +25,7 @@ var hipchat = { } } }; -var path = require("path"), - fs = require("fs-extra"), - glob = require("glob"), - _ = require('lodash'), - request = require("superagent"), - async = require("async"), - tarball = require('tarball-extract'), - mkdirp = require('mkdirp'); + hipchat.message('gray', 'Auto Update Started'); var newVersionCount = 0; var parse = function (json_file, ignore_missing, ignore_parse_fail) { @@ -48,9 +49,18 @@ var parse = function (json_file, ignore_missing, ignore_parse_fail) { } } +/** + * Check if an npmFileMap object contains any path which are not normalized, and thus could allow access to parent dirs + * @param pkg + * @returns {*} + */ var isValidFileMap = function(pkg){ var isValidPath = function(p){ - return p !== null && !p.match(/([\//]\.\.[\//])/); //don't allow parent dir access + if(p !== null){ //don't allow parent dir access, or tricky paths + p = p.replace(/\/+/g, '/'); //dont penalize for consequtive path seperators + return p === path.normalize(p); + } + return false }; if(pkg && pkg.npmFileMap){ @@ -74,8 +84,12 @@ var error = function(msg, name){ error.PKG_NAME = 'BadPackageName' error.FILE_PATH = 'BadFilePath' - +/** +* returns a fucntion that takes N args, where each arg is a path that must not outside of libPath. + * returns true if all paths are within libPath, else false +*/ var isAllowedPathFn = function(libPath){ //is path within the lib dir? if not, they shouldnt be writing/reading there + libPath = path.normalize(libPath); return function(){ var paths = 1 <= arguments.length ? [].slice.call(arguments, 0) : []; var re = new RegExp("^"+libPath) @@ -86,10 +100,18 @@ var isAllowedPathFn = function(libPath){ //is path within the lib dir? if not, t } }; -var processNewVersion = function(pkg, libPath, folderName){ + +/** + * Attempt to update the npmFileMap from extracted package.json, then using npmFileMap move required files to libPath/../ + * If the npmFileMap tries to modify files outside of libPath, dont let it! + * @param pkg + * @param libPath = root folder for extracted lib + * @returns {Array} = array of security related errors triggered during operation. + */ +var processNewVersion = function(pkg, libPath){ var isAllowedPath = isAllowedPathFn(libPath); - var newPkg = parse(path.join(libPath, folderName, 'package.json')); + var newPkg = parse(path.join(libPath, 'package.json')); if(isValidFileMap(newPkg)){ pkg.npmFileMap = newPkg.npmFileMap; } @@ -100,7 +122,7 @@ var processNewVersion = function(pkg, libPath, folderName){ var basePath = fileSpec.basePath || ""; _.each(fileSpec.files, function(file) { - var libContentsPath = path.normalize(path.join(libPath, folderName, basePath, file)); + var libContentsPath = path.normalize(path.join(libPath, basePath, file)); if(!isAllowedPath(libContentsPath)){ errors.push(error(pkg.npmName+" contains a malicious file path: "+libContentsPath, error.FILE_PATH)); return @@ -110,7 +132,7 @@ var processNewVersion = function(pkg, libPath, folderName){ _.each(files, function(extractFilePath) { if(extractFilePath.match(/(dependencies|\.zip\s*$)/i)) return; - var replacePath = path.normalize(path.join(folderName, basePath)); + var replacePath = path.normalize(path.join('package', basePath)); var actualPath = extractFilePath.replace(replacePath, ""); if(!isAllowedPath(extractFilePath, actualPath)){ errors.push(error(pkg.npmName+" contains a malicious file path: "+extractFilePath+' or '+actualPath, error.FILE_PATH)); @@ -123,6 +145,14 @@ var processNewVersion = function(pkg, libPath, folderName){ return errors; } +/** + * download and extract a tarball for a single npm version, get the files in npmFileMap and delete the rest + * @param pkg + * @param tarballUrl + * @param version + * @param cb + * @returns {*} + */ var updateLibraryVersion = function(pkg, tarballUrl, version, cb) { if(~pkg.name.indexOf("..")){ return cb(error(pkg.npmName+" has a malicious package name:"+ pkg.name, error.PKG_NAME)); @@ -135,13 +165,10 @@ var updateLibraryVersion = function(pkg, tarballUrl, version, cb) { var url = tarballUrl; var downloadFile = libPath + '/dist.tar.gz'; tarball.extractTarballDownload(url , downloadFile, libPath, {}, function(err, result) { - fs.unlinkSync(downloadFile); - var folderName = fs.readdirSync(libPath)[0]; - processNewVersion(pkg, libPath, folderName); - - fs.removeSync(path.join(libPath, folderName)); - + var extractPath = path.join(libPath, 'package'); + processNewVersion(pkg, extractPath); + fs.removeSync(extractPath); newVersionCount++; console.log("Do not have version", version, "of", pkg.npmName); cb() @@ -150,11 +177,18 @@ var updateLibraryVersion = function(pkg, tarballUrl, version, cb) { cb() } }; -var updateLibrary = function (pkg, callback) { + +/** + * grab all versions of a lib that has an 'npmFileMap' and 'npmName' in its package.json + * @param pkg + * @param tarballUrl + * @param cb + */ +var updateLibrary = function (pkg, cb) { if(!isValidFileMap(pkg)){ console.log(pkg.npmName+" has a malicious npmFileMap"); - hipchat.message('red', pkg.npmName+" has a malicious npmFileMap"); - return callback(null); + hipchat.message('red', pkg.npmName+" has a malicious npmFileMap: "+ JSON.stringify(pkg.npmFileMap)); + return cb(null); } console.log('Checking versions for ' + pkg.npmName); request.get('http://registry.npmjs.org/' + pkg.npmName, function(result) { @@ -167,23 +201,40 @@ var updateLibrary = function (pkg, callback) { pkg.version = npmVersion; fs.writeFileSync('ajax/libs/' + pkg.name + '/package.json', JSON.stringify(pkg, null, 2), 'utf8'); - callback(null); + cb(null); }); }); } -console.log('Looking for npm enabled libraries...'); +exports.run = function(){ + console.log('Looking for npm enabled libraries...'); -// load up those files -var packages = glob.sync("./ajax/libs/*/package.json"); -packages = _(packages).map(function (pkg) { - var parsedPkg = parse(pkg); - return (parsedPkg.npmName && parsedPkg.npmFileMap) ? parsedPkg : null; -}).compact().value(); -hipchat.message('green', 'Found ' + packages.length + ' npm enabled libraries'); -console.log('Found ' + packages.length + ' npm enabled libraries'); + // load up those files + var packages = glob.sync("./ajax/libs/*/package.json"); + packages = _(packages).map(function (pkg) { + var parsedPkg = parse(pkg); + return (parsedPkg.npmName && parsedPkg.npmFileMap) ? parsedPkg : null; + }).compact().value(); + hipchat.message('green', 'Found ' + packages.length + ' npm enabled libraries'); + console.log('Found ' + packages.length + ' npm enabled libraries'); + + async.eachSeries(packages, updateLibrary, function(err) { + console.log('Script completed'); + hipchat.message('green', 'Auto Update Completed - ' + newVersionCount + ' versions were updated'); + }); +} +exports.updateLibrary = updateLibrary; +exports.updateLibraryVersion = updateLibraryVersion; +exports.processNewVersion = processNewVersion; +exports.error = error; +exports.isAllowedPathFn = isAllowedPathFn; +exports.isValidFileMap = isValidFileMap; + + +var args = process.argv.slice(2); +if(args.length > 0 && args[0] == 'run'){ + exports.run() +} else { + console.log('to start, pass the "run" arg') +} -async.eachSeries(packages, updateLibrary, function(err) { - console.log('Script completed'); - hipchat.message('green', 'Auto Update Completed - ' + newVersionCount + ' versions were updated'); -}); diff --git a/auto-update.sh b/auto-update.sh index 24a28a512..84dc58f22 100755 --- a/auto-update.sh +++ b/auto-update.sh @@ -10,7 +10,7 @@ echo npm install for good measure /usr/local/bin/npm install echo Starting auto update script -/usr/local/bin/node auto-update.js >> node.log +/usr/local/bin/node auto-update.js run >> node.log echo Pushing new versions git add . From 38f03f4a15d728ac61237c723bf49064407ca850 Mon Sep 17 00:00:00 2001 From: Misha Koryak Date: Fri, 18 Apr 2014 18:26:48 -0400 Subject: [PATCH 5/6] add a few tests, need more --- auto-update.js | 16 ++++-- test/auto-update-test.js | 107 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/auto-update.js b/auto-update.js index 77b31fb09..dc41945f9 100644 --- a/auto-update.js +++ b/auto-update.js @@ -49,6 +49,10 @@ var parse = function (json_file, ignore_missing, ignore_parse_fail) { } } +var reEscape = function(s){ + return s.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'); +} + /** * Check if an npmFileMap object contains any path which are not normalized, and thus could allow access to parent dirs * @param pkg @@ -65,7 +69,7 @@ var isValidFileMap = function(pkg){ if(pkg && pkg.npmFileMap){ return _.every(pkg.npmFileMap, function(fileSpec){ - if(isValidPath(fileSpec.basePath || "")){ + if(isValidPath(fileSpec.basePath || "/")){ return _.every(fileSpec.files, isValidPath); } return false; @@ -89,10 +93,10 @@ error.FILE_PATH = 'BadFilePath' * returns true if all paths are within libPath, else false */ var isAllowedPathFn = function(libPath){ //is path within the lib dir? if not, they shouldnt be writing/reading there - libPath = path.normalize(libPath); + libPath = path.normalize(libPath || "/"); return function(){ var paths = 1 <= arguments.length ? [].slice.call(arguments, 0) : []; - var re = new RegExp("^"+libPath) + var re = new RegExp("^"+reEscape(libPath)); return _.every(paths, function(p) { p = path.normalize(p); return p.match(re); @@ -100,6 +104,9 @@ var isAllowedPathFn = function(libPath){ //is path within the lib dir? if not, t } }; +var invalidNpmName = function(name){ + return !!~name.indexOf(".."); //doesnt contain +} /** * Attempt to update the npmFileMap from extracted package.json, then using npmFileMap move required files to libPath/../ @@ -154,7 +161,7 @@ var processNewVersion = function(pkg, libPath){ * @returns {*} */ var updateLibraryVersion = function(pkg, tarballUrl, version, cb) { - if(~pkg.name.indexOf("..")){ + if(invalidNpmName(pkg.name)){ return cb(error(pkg.npmName+" has a malicious package name:"+ pkg.name, error.PKG_NAME)); } var libPath = path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); @@ -229,6 +236,7 @@ exports.processNewVersion = processNewVersion; exports.error = error; exports.isAllowedPathFn = isAllowedPathFn; exports.isValidFileMap = isValidFileMap; +exports.invalidNpmName = invalidNpmName; var args = process.argv.slice(2); diff --git a/test/auto-update-test.js b/test/auto-update-test.js index 38eb8ce94..bcb640084 100644 --- a/test/auto-update-test.js +++ b/test/auto-update-test.js @@ -1,8 +1,109 @@ var assert = require("assert"), path = require("path"), fs = require("fs"), - glob = require("glob"), vows = require("vows-si"), - jsv = require("JSV").JSV.createEnvironment(); + _ = require('lodash'), + au = require('./../auto-update'); -//TODO \ No newline at end of file + + + +var suite = vows.describe('NPM Auto Update - stand alone methods'); +suite.addBatch({ + 'npm name validation': { + topic: ["floatthead", "../evil"], + 'This is a valid npm name': function (arr) { + assert.equal(au.invalidNpmName(arr[0]), false); + }, + 'This is an invalid npm name': function (arr) { + assert.equal(au.invalidNpmName(arr[1]), true); + } + }, + 'npmFileMap validation - simple': { + topic: {"npmFileMap": [ + { + "basePath": "/dist/", + "files": [ + "*.js", + "blee/blah//script.js", + "blee/blah//script.min.js", + "styles.css", + "/test/**/*.*" + ] + } + ]}, + 'This is a valid npm file map': function (obj) { + assert.equal(au.isValidFileMap(obj), true); + }, + 'file paths are ok too': function(obj){ + var map = obj.npmFileMap[0]; + var testFn = au.isAllowedPathFn(path.join('someplace', map.basePath)); + assert.equal(testFn.apply(null, _.map(map.files, function(f){ return path.join("someplace", map.basePath, f)})), true) + } + }, + 'npmFileMap validation - arrays': { + topic: {"npmFileMap": [ + { + "basePath": "/dist/", + "files": [ + "*.js", + "blee/blah//script.js", + "blee/blah//script.min.js", + "styles.css", + "/test/**/*.*" + ] + }, + { + "basePath": "", + "files": [ + "test.css", + "/blee.js", + "this_is_ok_right_now.zip" + ] + }, + { + "basePath": "/", + "files": [ + "*" + ] + } + ]}, + 'valid array of file maps': function (obj) { + assert.equal(au.isValidFileMap(obj), true); + }, + 'these paths are ok too': function(obj){ + var map = obj.npmFileMap[0]; + var testFn = au.isAllowedPathFn(path.join('someplace', map.basePath)); + assert.equal(testFn.apply(null, _.map(map.files, function(f){ return path.join("someplace", map.basePath, f)})), true) + }, + 'these paths are also allowed': function(obj){ + var map = obj.npmFileMap[1]; + var testFn = au.isAllowedPathFn(path.join('someplace', map.basePath)); + assert.equal(testFn.apply(null, _.map(map.files, function(f){ return path.join("someplace", map.basePath, f)})), true) + } + + }, + 'npmFileMap validation - invalid 1': { + topic: {"npmFileMap": [ + { + "basePath": "/dist/", + "files": [ + "*.js", + "blee/blah/../../../script.js", + "/../../../../../../../../../../etc/hosts", + "styles.css", + "/test/**/*.*" + ] + } + ]}, + 'this npm filemap is doing evil things': function (obj) { + assert.equal(au.isValidFileMap(obj), false); + }, + 'these paths are bad': function(obj){ + var map = obj.npmFileMap[0]; + var testFn = au.isAllowedPathFn(path.join('someplace', map.basePath)); + assert.equal(testFn.apply(null, _.map(map.files, function(f){ return path.join("someplace", map.basePath, f)})), false) + } + } +}); +suite.export(module); From 3ecf98a9d67e0ae9b8a5f97ece926b8690f5320c Mon Sep 17 00:00:00 2001 From: Misha Koryak Date: Tue, 22 Apr 2014 13:21:02 -0400 Subject: [PATCH 6/6] disabled grabbing npmFileMap from other package json files (for now) made the script copy files into a temp dir and then move to /ajax/libs/* which makes it work better see #3036 --- auto-update.js | 82 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/auto-update.js b/auto-update.js index dc41945f9..c4e284bfb 100644 --- a/auto-update.js +++ b/auto-update.js @@ -1,5 +1,6 @@ var Hipchat = require('node-hipchat'), path = require("path"), + assert = require("assert"), fs = require("fs-extra"), glob = require("glob"), _ = require('lodash'), @@ -30,7 +31,7 @@ hipchat.message('gray', 'Auto Update Started'); var newVersionCount = 0; var parse = function (json_file, ignore_missing, ignore_parse_fail) { var content; - + try { content = fs.readFileSync(json_file, 'utf8'); } catch (err1) { @@ -89,9 +90,9 @@ error.PKG_NAME = 'BadPackageName' error.FILE_PATH = 'BadFilePath' /** -* returns a fucntion that takes N args, where each arg is a path that must not outside of libPath. + * returns a fucntion that takes N args, where each arg is a path that must not outside of libPath. * returns true if all paths are within libPath, else false -*/ + */ var isAllowedPathFn = function(libPath){ //is path within the lib dir? if not, they shouldnt be writing/reading there libPath = path.normalize(libPath || "/"); return function(){ @@ -115,12 +116,18 @@ var invalidNpmName = function(name){ * @param libPath = root folder for extracted lib * @returns {Array} = array of security related errors triggered during operation. */ -var processNewVersion = function(pkg, libPath){ - var isAllowedPath = isAllowedPathFn(libPath); +var processNewVersion = function(pkg, version){ + var extractLibPath = path.join(getPackageTempPath(pkg, version), 'package'); + var libPath = getPackagePath(pkg, version) - var newPkg = parse(path.join(libPath, 'package.json')); - if(isValidFileMap(newPkg)){ - pkg.npmFileMap = newPkg.npmFileMap; + var isAllowedPath = isAllowedPathFn(extractLibPath); + + var newPath = path.join(libPath, 'package.json') + if(false && fs.existsSync(newPath)){ //turn this off for now + var newPkg = parse(newPath); + if(isValidFileMap(newPkg)){ + pkg.npmFileMap = newPkg.npmFileMap; + } } var npmFileMap = pkg.npmFileMap; var errors = []; @@ -129,29 +136,37 @@ var processNewVersion = function(pkg, libPath){ var basePath = fileSpec.basePath || ""; _.each(fileSpec.files, function(file) { - var libContentsPath = path.normalize(path.join(libPath, basePath, file)); + var libContentsPath = path.normalize(path.join(extractLibPath, basePath)); if(!isAllowedPath(libContentsPath)){ errors.push(error(pkg.npmName+" contains a malicious file path: "+libContentsPath, error.FILE_PATH)); return } - var files = glob.sync(libContentsPath); + var files = glob.sync(path.join(libContentsPath, file)); + var copyPath = path.join(libPath, basePath) _.each(files, function(extractFilePath) { if(extractFilePath.match(/(dependencies|\.zip\s*$)/i)) return; - var replacePath = path.normalize(path.join('package', basePath)); - var actualPath = extractFilePath.replace(replacePath, ""); - if(!isAllowedPath(extractFilePath, actualPath)){ - errors.push(error(pkg.npmName+" contains a malicious file path: "+extractFilePath+' or '+actualPath, error.FILE_PATH)); - return; - } - fs.renameSync(extractFilePath, actualPath); + var copyPart = path.relative(libContentsPath, extractFilePath); + var copyPath = path.join(libPath, copyPart) + fs.mkdirsSync(path.dirname(copyPath)) + //TODO remove me: + console.log('rename:',extractFilePath, copyPath) + + fs.renameSync(extractFilePath, copyPath); }); }); }); return errors; } + +var getPackageTempPath = function(pkg, version){ + return path.normalize(path.join(__dirname, 'temp', pkg.name, version)) +} +var getPackagePath = function(pkg, version){ + return path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); +} /** * download and extract a tarball for a single npm version, get the files in npmFileMap and delete the rest * @param pkg @@ -164,20 +179,22 @@ var updateLibraryVersion = function(pkg, tarballUrl, version, cb) { if(invalidNpmName(pkg.name)){ return cb(error(pkg.npmName+" has a malicious package name:"+ pkg.name, error.PKG_NAME)); } - var libPath = path.normalize(path.join(__dirname, 'ajax', 'libs', pkg.name, version)); + var extractLibPath = getPackageTempPath(pkg, version); + var libPath = getPackagePath(pkg, version); if(!fs.existsSync(libPath)) { - fs.mkdirSync(libPath); + fs.mkdirsSync(extractLibPath); var url = tarballUrl; - var downloadFile = libPath + '/dist.tar.gz'; - tarball.extractTarballDownload(url , downloadFile, libPath, {}, function(err, result) { - fs.unlinkSync(downloadFile); - var extractPath = path.join(libPath, 'package'); - processNewVersion(pkg, extractPath); - fs.removeSync(extractPath); - newVersionCount++; - console.log("Do not have version", version, "of", pkg.npmName); + var downloadFile = path.join(extractLibPath, 'dist.tar.gz'); + tarball.extractTarballDownload(url , downloadFile, extractLibPath, {}, function(err, result) { + if(fs.existsSync(downloadFile)){ + processNewVersion(pkg, version); + newVersionCount++; + console.log("Do not have version", version, "of", pkg.npmName); + } else { + console.log("error downloading "+ version+ "of "+pkg.npmName+" it didnt exist: ", result, err) + } cb() }); } else { @@ -207,13 +224,13 @@ var updateLibrary = function (pkg, cb) { var npmVersion = result.body['dist-tags'] && result.body['dist-tags'].latest || 0; pkg.version = npmVersion; fs.writeFileSync('ajax/libs/' + pkg.name + '/package.json', JSON.stringify(pkg, null, 2), 'utf8'); - cb(null); }); }); } exports.run = function(){ + fs.removeSync(path.join(__dirname, 'temp')) console.log('Looking for npm enabled libraries...'); // load up those files @@ -226,8 +243,9 @@ exports.run = function(){ console.log('Found ' + packages.length + ' npm enabled libraries'); async.eachSeries(packages, updateLibrary, function(err) { - console.log('Script completed'); - hipchat.message('green', 'Auto Update Completed - ' + newVersionCount + ' versions were updated'); + console.log('Script completed'); + hipchat.message('green', 'Auto Update Completed - ' + newVersionCount + ' versions were updated'); + fs.removeSync(path.join(__dirname, 'temp')) }); } exports.updateLibrary = updateLibrary; @@ -241,8 +259,8 @@ exports.invalidNpmName = invalidNpmName; var args = process.argv.slice(2); if(args.length > 0 && args[0] == 'run'){ - exports.run() + exports.run() } else { - console.log('to start, pass the "run" arg') + console.log('to start, pass the "run" arg') }