Browse Source

Guard against injection in some file system calls.

bentleyjoakes 6 years ago
parent
commit
260a30f6f2
2 changed files with 90 additions and 67 deletions
  1. 67 62
      ___fs++.js
  2. 23 5
      httpwsd.js

+ 67 - 62
___fs++.js

@@ -9,8 +9,8 @@
   	provide higher-level functions that those provided by nodejs' fs module */
 
 var _cp = require('child_process'),
-	 _os = require('os'),
-     _fs = require('fs');
+	_os = require('os'),
+	_fs = require('fs');
 
 
 /* NOTE:: because microsoft has apparently diversified into hiring comedians,
@@ -18,6 +18,10 @@ var _cp = require('child_process'),
 exports.cp =
 	function(src,dest,callback)
 	{
+		//guard against injection
+		src = src.split(';')[0];
+		dest = dest.split(';')[0];
+
 		switch(_os.type())
 		{
 			case 'Windows_NT' :
@@ -30,7 +34,7 @@ exports.cp =
 							callback(err,stdout,stderr);
 					});
 				break;
-				
+
 			case 'Linux'  :
 			case 'Darwin' :
 				_cp.exec('cp -R "'+src+'" "'+dest+'"',callback);
@@ -51,6 +55,9 @@ exports.cp =
 exports.findfiles =
 	function(dir,callback)
 	{
+		//guard against injection
+		dir = dir.split(';')[0];
+
 		switch(_os.type())
 		{
 			case 'Windows_NT' :
@@ -61,30 +68,30 @@ exports.findfiles =
 							callback(err,stdout,stderr);
 						else
 						{
-							var windir = (dir.charAt(0)=='.' ? dir.substring(1) : dir).
-												replace(/\//g,'\\'),
-								 paths  = stdout.split('\r\n').map(
+							let windir = (dir.charAt(0)=='.' ? dir.substring(1) : dir).
+								replace(/\//g,'\\'),
+								paths  = stdout.split('\r\n').map(
 									function(path)
 									{
-										var newpath = dir+path.substring(
-														path.indexOf(windir)+windir.length).
-													  replace(/\\/g,'/');
-                                        try {
-                                            if (_fs.lstatSync(path).isDirectory()) {
-                                                newpath = newpath + '/';
-                                            }
-                                        } catch (e) {
-                                            console.log("Error!");
-                                            console.log(e);
+										let newpath = dir+path.substring(
+											path.indexOf(windir)+windir.length).
+										replace(/\\/g,'/');
+										try {
+											if (_fs.lstatSync(path).isDirectory()) {
+												newpath = newpath + '/';
+											}
+										} catch (e) {
+											console.log("Error!");
+											console.log(e);
 										}
-                                        return newpath;
+										return newpath;
 									});
 							paths.pop();
 							callback(err,paths.join('\n'),stderr);
 						}
 					});
 				break;
-				
+
 			case 'Linux'  :
 			case 'Darwin' :
 				_cp.exec('find "'+dir+'"',
@@ -93,14 +100,14 @@ exports.findfiles =
 						if( err )
 							callback(err,stdout,stderr);
 						else {
-                            var paths = stdout.slice(0,-1),
-                                newpaths = paths.split('\n').map(function(path) {
-                                    if (_fs.lstatSync(path).isDirectory()) {
-                                        return path + "/";
-                                    } else return path;
-                                });
+							let paths = stdout.slice(0,-1),
+								newpaths = paths.split('\n').map(function(path) {
+									if (_fs.lstatSync(path).isDirectory()) {
+										return path + "/";
+									} else return path;
+								});
 							callback(err,newpaths.join('\n'),stderr);
-                        }
+						}
 					});
 				break;
 
@@ -116,15 +123,18 @@ exports.findfiles =
 exports.mkdirs =
 	function(dir,callback)
 	{
-        var split_dir = dir.split('/'),
-            curr_dir = '';
-        for (var i in split_dir) {
-            curr_dir += split_dir[i] + '/';
-            if (!_fs.existsSync(curr_dir)) {
-                _fs.mkdir(curr_dir, 484, function(err) {if (err) {if (err.code != 'EEXIST') callback(err);}});
-            }
-        }
-        callback();
+		//guard against injection
+		dir = dir.split(';')[0];
+
+		let split_dir = dir.split('/'),
+			curr_dir = '';
+		for (let i in split_dir) {
+			curr_dir += split_dir[i] + '/';
+			if (!_fs.existsSync(curr_dir)) {
+				_fs.mkdir(curr_dir, 484, function(err) {if (err) {if (err.code != 'EEXIST') callback(err);}});
+			}
+		}
+		callback();
 	};
 
 
@@ -132,8 +142,12 @@ exports.mkdirs =
 exports.mv =
 	function(src,dest,callback)
 	{
-        var split_string = src.split('/');
-        _fs.rename(src, dest + split_string[split_string.length-1],callback);
+		//guard against injection
+		src = src.split(';')[0];
+		dest = dest.split(';')[0];
+
+		let split_string = src.split('/');
+		_fs.rename(src, dest + split_string[split_string.length-1],callback);
 	};
 
 
@@ -141,37 +155,28 @@ exports.mv =
 exports.rmdirs =
 	function(dir,callback)
 	{
-		switch(_os.type())
-		{
-			case 'Windows_NT' :
-				_cp.exec('rmdir /s "'+dir+'"',callback);
-				break;
-				
-			case 'Linux'  :
-			case 'Darwin' :
-				_cp.exec('rm -rf "'+dir+'"',callback);
-				break;
+		//guard against injection
+		dir = dir.split(';')[0];
 
-			default:
-				throw 'unsupported OS :: '+_os.type();
-		}
+		_fs.rmdir(dir, callback)
 	};
 
-
-
 // http://stackoverflow.com/questions/18052762/remove-directory-which-is-not-empty
 exports.deleteFolderRecursive = function(path) {
-   if( _fs.existsSync(path) ) {
-       _fs.readdirSync(path).forEach(function(file,index){
-           var curPath = path + "/" + file;
-           if(_fs.lstatSync(curPath).isDirectory()) { // recurse
-               exports.deleteFolderRecursive(curPath);
-           } else { // delete file
-               _fs.unlinkSync(curPath);
-           }
-       });
-       _fs.rmdirSync(path);
-   }
+	//guard against injection
+	path = path.split(';')[0];
+
+	if( _fs.existsSync(path) ) {
+		_fs.readdirSync(path).forEach(function(file,index){
+			let curPath = path + "/" + file;
+			if(_fs.lstatSync(curPath).isDirectory()) { // recurse
+				exports.deleteFolderRecursive(curPath);
+			} else { // delete file
+				_fs.unlinkSync(curPath);
+			}
+		});
+		_fs.rmdirSync(path);
+	}
 };
 	
 	

+ 23 - 5
httpwsd.js

@@ -542,9 +542,16 @@ var httpserver = _http.createServer(
 			{
 				req.setEncoding('utf8');
 
-				var reqData = '',
+				let reqData = '',
 					 tmpzip	= 'upload'+Date.now()+'.zip',
 					 destdir	= './users/'+url.pathname.match(/(.*)\.file$/)[1]+'/';
+
+				if( url.pathname.contains("..") || url.pathname.contains(";") ) {
+					__respond(resp, 404,
+						'invalid pathname, no semicolons or .. allowed :: ' + url.pathname);
+					return;
+				}
+
 				req.addListener("data", function(chunk) {reqData += chunk;});
 				req.addListener("end", 
 					function() 
@@ -570,7 +577,10 @@ var httpserver = _http.createServer(
 														__respond(resp,500,String(err));
 													else
 														__respond(resp,200);
-													_fs.unlink(destdir+tmpzip);
+													_fs.unlink(destdir+tmpzip, function(err){
+														console.log("Unlinked " + destdir + tmpzip);
+														}
+													);
 												});
 										});
 							});
@@ -580,12 +590,18 @@ var httpserver = _http.createServer(
 			/* serve specified file/folder within a zip file */ 
 			else if( req.method == 'GET' && url.pathname.match(/\.file$/) )
 			{
-				var matches  = url.pathname.match(/^\/(.*?)\/(.*)\.file$/),
+				let matches  = url.pathname.match(/^\/(.*?)\/(.*)\.file$/),
 					 username = matches[1],
  					 fname 	 = './'+matches[2],
 					 userdir	 = './users/'+username+'/',
 					 tmpzip	 = 'download'+Date.now()+'.zip';
 
+				if( username.contains("..") || username.contains(";") ) {
+					__respond(resp, 404,
+						'invalid username, no colons or .. allowed :: ' + username);
+					return;
+				}
+
 				_fs.exists(userdir+fname,
 					function(exists)
 					{
@@ -599,14 +615,16 @@ var httpserver = _http.createServer(
 									if( err )
 										__respond(resp,500,String(err));
 									else
-										_fs.readFile(userdir+tmpzip,'binary',
+										_fs.readFile(userdir+tmpzip,
 											function(err, data)
 											{
 												__respond(resp,200,'',data,
 													{'Content-Type':'application/zip',
 													 'Content-Disposition':
 													 	'attachment; filename="'+tmpzip+'"'});
-												_fs.unlink(userdir+tmpzip);
+												_fs.unlink(userdir+tmpzip, function(err){
+													console.log("Unlinked " + userdir+tmpzip);
+												});
 											});
 								});
 					});