diff --git a/lib/request-guards.js b/lib/request-guards.js new file mode 100644 index 0000000..954a956 --- /dev/null +++ b/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; +}; diff --git a/package.json b/package.json index 353d50a..916a48e 100644 --- a/package.json +++ b/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": "", diff --git a/server.js b/server.js index 0810d1c..476ef8c 100644 --- a/server.js +++ b/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); }); diff --git a/test/request-guards.test.js b/test/request-guards.test.js new file mode 100644 index 0000000..1cb78de --- /dev/null +++ b/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); +});