fix: harden image paths and upload ids

lib/request-guards.js

@@ -0,0 +1,41 @@
+var path = require('path');
+
+var imageRoot = path.resolve(__dirname, '..', 'public', 'images');
+
+exports.imageRoot = imageRoot;
+
+exports.normalizeCollectionId = function(collectionId) {
+  if (typeof collectionId !== 'string') {
+    return null;
+  }
+
+  var trimmed = collectionId.trim();
+  if (!/^\d+$/.test(trimmed)) {
+    return null;
+  }
+
+  return trimmed;
+};
+
+exports.resolveImagePath = function(pathname) {
+  var decodedPath;
+
+  try {
+    decodedPath = decodeURIComponent(pathname);
+  } catch (err) {
+    return null;
+  }
+
+  if (decodedPath.indexOf('/public/images/') !== 0) {
+    return null;
+  }
+
+  var relativePath = decodedPath.replace(/^\/public\/images\//, '');
+  var resolvedPath = path.resolve(imageRoot, relativePath);
+
+  if (resolvedPath.indexOf(imageRoot + path.sep) !== 0) {
+    return null;
+  }
+
+  return resolvedPath;
+};

package.json

@@ -3,7 +3,7 @@
   "version": "1.0.0",
   "main": "server.js",
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1",
+    "test": "node --test test/request-guards.test.js",
     "start": "node server.js"
   },
   "author": "",

server.js

@@ -1,11 +1,12 @@
 var http = require('http');
 var url = require('url');
-var join = require('path').join;
+var path = require('path');
 var qs = require('querystring');
 var fs = require('fs');
 var mkdirp = require('mkdirp');
 var formidable = require('formidable');
 var fileServer = require('./lib/fileserver.js');
+var guards = require('./lib/request-guards.js');
 var mysql = require('mysql');
 var db = mysql.createConnection({
 	host: '127.0.0.1',
@@ -22,9 +23,18 @@ http.createServer(function(req, resp) {
 		case 'GET':
 			var new_uri = url.parse(req.url);
 			if(new_uri.pathname.indexOf('/public/images')== 0){
-				// console.log(new_uri);
-				fs.createReadStream('.'+new_uri.pathname).pipe(resp);
-				// resp.end("a phto");
+				var imagePath = guards.resolveImagePath(new_uri.pathname);
+				if (!imagePath) {
+					resp.statusCode = 400;
+					resp.end('Invalid image path');
+					return;
+				}
+				fs.createReadStream(imagePath)
+					.on('error', function() {
+						resp.statusCode = 404;
+						resp.end('Image not found');
+					})
+					.pipe(resp);
 				return;	
 			}
 			if(new_uri.pathname == '/api' ){
@@ -97,9 +107,13 @@ http.createServer(function(req, resp) {
 				return;	
 			}
 			if(new_post_uri.pathname.indexOf('/listphoto')== 0){
-				// console.log(new_post_uri.pathname.split("/")[2]);
-
-				fileServer.listPhoto(db, new_post_uri.pathname.split("/")[2], resp);
+				var collectionNo = guards.normalizeCollectionId(new_post_uri.pathname.split("/")[2]);
+				if (!collectionNo) {
+					resp.statusCode = 400;
+					resp.end('Invalid collection id');
+					return;
+				}
+				fileServer.listPhoto(db, collectionNo, resp);
 				return;	
 			}
 
@@ -112,17 +126,29 @@ http.createServer(function(req, resp) {
 				form.parse(req, function (err, fields, files) {
 					if(err)throw err;
 					// console.log("Successfully formidable");
-					db.query("select * from photographs where collection_number = ?", fields.file_details, function (err, row) {
+					var collectionId = guards.normalizeCollectionId(fields.file_details);
+					if (!collectionId) {
+						resp.statusCode = 400;
+						resp.end('Invalid collection id');
+						return;
+					}
+					if (!files.uploaded_file || !files.uploaded_file.path) {
+						resp.statusCode = 400;
+						resp.end('Missing upload');
+						return;
+					}
+					db.query("select * from photographs where collection_number = ?", collectionId, function (err, row) {
 						if(err) throw err;
 						// console.log(row);
-						var new_url = "public/images/"+fields.file_details;
+						var new_url = path.join("public", "images", collectionId);
 						mkdirp(new_url, function (err) {
 							if(err)throw err;
 							// console.log("Successfully made dir");
-							fs.rename(files.uploaded_file.path, new_url+"/"+row.length+".jpg", function (err) {
+							var photoPath = path.join(new_url, row.length + ".jpg");
+							fs.rename(files.uploaded_file.path, photoPath, function (err) {
 								if (err) throw err;
 								// console.log("Successfully Uploaded");
-								fileServer.addPhoto(db, new_url+"/"+row.length+".jpg", fields.file_details, resp);
+								fileServer.addPhoto(db, photoPath, collectionId, resp);
 							});
 							// console.log(files.uploaded_file.name);
 						});

test/request-guards.test.js

@@ -0,0 +1,29 @@
+var test = require('node:test');
+var assert = require('node:assert/strict');
+var path = require('path');
+var guards = require('../lib/request-guards.js');
+
+test('normalizeCollectionId accepts numeric collection ids', function() {
+  assert.equal(guards.normalizeCollectionId('42'), '42');
+  assert.equal(guards.normalizeCollectionId(' 42 '), '42');
+});
+
+test('normalizeCollectionId rejects unsafe collection ids', function() {
+  assert.equal(guards.normalizeCollectionId('../1'), null);
+  assert.equal(guards.normalizeCollectionId('1/2'), null);
+  assert.equal(guards.normalizeCollectionId('abc'), null);
+});
+
+test('resolveImagePath keeps image paths inside public images', function() {
+  var resolvedPath = guards.resolveImagePath('/public/images/42/0.jpg');
+
+  assert.equal(
+    resolvedPath,
+    path.join(guards.imageRoot, '42', '0.jpg'),
+  );
+});
+
+test('resolveImagePath rejects traversal outside public images', function() {
+  assert.equal(guards.resolveImagePath('/public/images/../form.html'), null);
+  assert.equal(guards.resolveImagePath('/public/images/%2e%2e/form.html'), null);
+});