Skip to content

Commit

Permalink
Sanitize dynamic path.resolve and path.join calls
Browse files Browse the repository at this point in the history
  • Loading branch information
timkendrick committed Apr 5, 2016
1 parent 566cd59 commit 2e3e8fc
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 20 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"handlebars": "^4.0.5",
"htmlbars": "^0.14.16",
"ini": "^1.3.4",
"is-path-inside": "^1.0.0",
"is-url": "^1.2.1",
"istextorbinary": "^1.0.2",
"jshashes": "^1.0.5",
Expand Down
10 changes: 6 additions & 4 deletions src/adapters/DropboxAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ var DropboxOAuth2Strategy = require('passport-dropbox-oauth2').Strategy;
var LoginAdapter = require('./LoginAdapter');
var StorageAdapter = require('./StorageAdapter');

var resolveChildPath = require('../utils/resolveChildPath');

var FileModel = require('../models/FileModel');

var HttpError = require('../errors/HttpError');
Expand Down Expand Up @@ -291,7 +293,7 @@ DropboxStorageAdapter.prototype.readFile = function(filePath, siteAdapterConfig,
.connect(uid, accessToken)
.then(function(dropboxClient) {
var sitePath = siteAdapterConfig.path;
var fullPath = path.join(sitePath, filePath);
var fullPath = resolveChildPath(sitePath, filePath);
return dropboxClient.readFile(fullPath);
});
};
Expand All @@ -305,7 +307,7 @@ DropboxStorageAdapter.prototype.retrieveDownloadLink = function(filePath, siteAd
.connect(uid, accessToken)
.then(function(dropboxClient) {
var sitePath = siteAdapterConfig.path;
var fullPath = path.join(sitePath, filePath);
var fullPath = resolveChildPath(sitePath, filePath);
return dropboxClient.generateDownloadLink(fullPath);
});
};
Expand All @@ -319,7 +321,7 @@ DropboxStorageAdapter.prototype.retrievePreviewLink = function(filePath, siteAda
.connect(uid, accessToken)
.then(function(dropboxClient) {
var sitePath = siteAdapterConfig.path;
var fullPath = path.join(sitePath, filePath);
var fullPath = resolveChildPath(sitePath, filePath);
return dropboxClient.generatePreviewLink(fullPath);
});
};
Expand All @@ -333,7 +335,7 @@ DropboxStorageAdapter.prototype.retrieveThumbnailLink = function(filePath, siteA
.connect(uid, accessToken)
.then(function(dropboxClient) {
var sitePath = siteAdapterConfig.path;
var fullPath = path.join(sitePath, filePath);
var fullPath = resolveChildPath(sitePath, filePath);
return dropboxClient.generateThumbnailLink(fullPath);
});
};
Expand Down
37 changes: 25 additions & 12 deletions src/adapters/LocalAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var StorageAdapter = require('./StorageAdapter');

var AuthenticationService = require('../services/AuthenticationService');

var resolveChildPath = require('../utils/resolveChildPath');
var loadFileMetadata = require('../utils/loadFileMetadata');

var HttpError = require('../errors/HttpError');
Expand Down Expand Up @@ -190,8 +191,8 @@ LocalStorageAdapter.prototype.initSiteFolder = function(siteFiles, siteAdapterCo


function checkWhetherFileExists(filePath) {
var fullPath = path.join(sitesRoot, filePath);
return new Promise(function(resolve, reject) {
var fullPath = resolveChildPath(sitesRoot, filePath);
fs.stat(fullPath, function(error, stat) {
if (error && (error.code === 'ENOENT')) {
return resolve(false);
Expand All @@ -206,8 +207,8 @@ LocalStorageAdapter.prototype.initSiteFolder = function(siteFiles, siteAdapterCo
function copySiteFiles(sitePath, dirContents) {
var files = getFileListing(dirContents);
return Promise.resolve(mapSeries(files, function(fileMetadata) {
var filePath = path.join(sitePath, fileMetadata.path);
var fullPath = path.join(sitesRoot, filePath);
var filePath = resolveChildPath(sitePath, fileMetadata.path);
var fullPath = resolveChildPath(sitesRoot, filePath);
var fileContents = fileMetadata.contents;
return writeFile(fullPath, fileContents);
}).then(function(results) {
Expand Down Expand Up @@ -249,11 +250,17 @@ LocalStorageAdapter.prototype.initSiteFolder = function(siteFiles, siteAdapterCo
LocalStorageAdapter.prototype.loadSiteContents = function(siteAdapterConfig, userAdapterConfig) {
var sitesRoot = this.sitesRoot;
var siteFolderPath = siteAdapterConfig.path;
var fullPath = path.join(sitesRoot, siteFolderPath);
return loadFileMetadata(fullPath, {
root: fullPath,
contents: true
return new Promise(function(resolve, reject) {
resolve(
resolveChildPath(sitesRoot, siteFolderPath)
);
})
.then(function(fullPath) {
return loadFileMetadata(fullPath, {
root: fullPath,
contents: true
});
})
.then(function(rootFolder) {
return {
root: rootFolder,
Expand All @@ -265,8 +272,8 @@ LocalStorageAdapter.prototype.loadSiteContents = function(siteAdapterConfig, use
LocalStorageAdapter.prototype.readFile = function(filePath, siteAdapterConfig, userAdapterConfig) {
var sitesRoot = this.sitesRoot;
var sitePath = siteAdapterConfig.path;
var fullPath = path.join(sitesRoot, sitePath, filePath);
return new Promise(function(resolve, reject) {
var fullPath = resolveChildPath(sitesRoot, sitePath, filePath);
fs.readFile(fullPath, { encoding: 'utf8' }, function(error, data) {
if (error) { return reject(error); }
resolve(data);
Expand Down Expand Up @@ -294,11 +301,17 @@ LocalStorageAdapter.prototype.retrieveThumbnailLink = function(filePath, siteAda

LocalStorageAdapter.prototype.retrieveFileMetadata = function(filePath, userAdapterConfig) {
var sitesRoot = this.sitesRoot;
var fullPath = path.resolve(sitesRoot, filePath);
return loadFileMetadata(fullPath, {
root: sitesRoot,
contents: false
return new Promise(function(resolve, reject) {
resolve(
resolveChildPath(sitesRoot, filePath)
);
})
.then(function(fullPath) {
return loadFileMetadata(fullPath, {
root: sitesRoot,
contents: false
});
})
.catch(function(error) {
if (error.code === 'ENOENT') { return null; }
throw error;
Expand Down
2 changes: 1 addition & 1 deletion src/apps/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ module.exports = function(database, cache, config) {
var siteTemplatePath = config.templates.site;
var themesPath = config.themes.root;

var partialsPath = path.resolve(appTemplatesPath, '_partials');
var partialsPath = path.join(appTemplatesPath, '_partials');
var adminTemplatesPath = path.join(appTemplatesPath, 'admin');
var demoTemplatesPath = path.join(appTemplatesPath, 'demo');
var faqPath = path.join(appTemplatesPath, 'faq/faq.json');
Expand Down
14 changes: 14 additions & 0 deletions src/utils/resolveChildPath.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

var path = require('path');
var isPathInside = require('is-path-inside');

var HttpError = require('../errors/HttpError');

module.exports = function(from, to) {
var parentPath = path.resolve(from);
var childPaths = Array.prototype.slice.call(arguments, 1);
var fullPath = path.join.apply(path, [parentPath].concat(childPaths));
if (!isPathInside(fullPath, parentPath)) { throw new HttpError(403, 'Invalid path'); }
return fullPath;
};
6 changes: 3 additions & 3 deletions src/utils/resolveThemePaths.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
'use strict';

var path = require('path');
var objectAssign = require('object-assign');

var resolveChildPath = require('./resolveChildPath');
var resolvePartials = require('./resolvePartials');

module.exports = function(theme, themePath) {
return objectAssign({}, theme, {
templates: Object.keys(theme.templates).reduce(function(templates, templateId) {
var template = theme.templates[templateId];
var templatePath = path.resolve(themePath, template.filename);
var partialsRoot = (template.options && template.options.partials ? path.resolve(themePath, template.options.partials) : null);
var templatePath = resolveChildPath(themePath, template.filename);
var partialsRoot = (template.options && template.options.partials ? resolveChildPath(themePath, template.options.partials) : null);
var resolvedPartials = (partialsRoot ? resolvePartials(partialsRoot) : null);
var resolvedOptions = (resolvedPartials ? objectAssign({}, template.options, { partials: resolvedPartials }) : template.options);
var resolvedTemplate = objectAssign({}, template, {
Expand Down

0 comments on commit 2e3e8fc

Please sign in to comment.