Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security related fixes #135

Merged
merged 1 commit into from
Jan 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .npmignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
*.yml
.editorconfig
/docs/
/artifacts/
/examples/
/tests/
27 changes: 16 additions & 11 deletions libs/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ var debug = require('debug')('Fetchr');
var fumble = require('fumble');
var objectAssign = require('object-assign');
var Promise = global.Promise || require('es6-promise').Promise;
var RESOURCE_SANTIZER_REGEXP = /[^\w\.]+/g;

function parseValue(value) {
// take care of value of type: array, object
Expand All @@ -38,6 +39,10 @@ function parseParamValues (params) {
}, {});
}

function sanitizeResourceName(resource) {
return resource ? resource.replace(RESOURCE_SANTIZER_REGEXP, '*') : resource;
}

/**
* Takes an error and resolves output and statusCode to respond to client with
*
Expand All @@ -56,7 +61,6 @@ function getErrorResponse(err) {
output.message = err.message;
}


return {
statusCode: statusCode,
output: output
Expand Down Expand Up @@ -267,7 +271,7 @@ Fetcher.getService = function (name) {
//Access service by name
var service = Fetcher.isRegistered(name);
if (!service) {
throw new Error('Service "' + name + '" could not be found');
throw new Error('Service "' + sanitizeResourceName(name) + '" could not be found');
}
return service;
};
Expand Down Expand Up @@ -304,19 +308,20 @@ Fetcher.middleware = function (options) {
return function (req, res, next) {
var request;
var error;
var serviceMeta;

if (req.method === GET) {
var path = req.path.substr(1).split(';');
var resource = path.shift();

if (!Fetcher.isRegistered(resource)) {
error = fumble.http.badRequest('Invalid Fetchr Access', {
debug: 'Bad resource ' + resource
debug: 'Bad resource ' + sanitizeResourceName(resource)
});
error.source = 'fetchr';
return next(error);
}
var serviceMeta = [];
serviceMeta = [];
request = new Request(OP_READ, resource, {
req: req,
serviceMeta: serviceMeta
Expand Down Expand Up @@ -359,12 +364,12 @@ Fetcher.middleware = function (options) {

if (!Fetcher.isRegistered(singleRequest.resource)) {
error = fumble.http.badRequest('Invalid Fetchr Access', {
debug: 'Bad resource ' + singleRequest.resource
debug: 'Bad resource ' + sanitizeResourceName(singleRequest.resource)
});
error.source = 'fetchr';
return next(error);
}
var serviceMeta = [];
serviceMeta = [];
request = new Request(singleRequest.operation, singleRequest.resource, {
req: req,
serviceMeta: serviceMeta
Expand Down Expand Up @@ -431,7 +436,7 @@ Fetcher.prototype.read = function (resource, params, config, callback) {
return request
.params(params)
.clientConfig(config)
.end(callback)
.end(callback);
};
/**
* create operation (create as in CRUD).
Expand Down Expand Up @@ -467,7 +472,7 @@ Fetcher.prototype.create = function (resource, params, body, config, callback) {
.params(params)
.body(body)
.clientConfig(config)
.end(callback)
.end(callback);
};
/**
* update operation (update as in CRUD).
Expand Down Expand Up @@ -503,7 +508,7 @@ Fetcher.prototype.update = function (resource, params, body, config, callback) {
.params(params)
.body(body)
.clientConfig(config)
.end(callback)
.end(callback);
};
/**
* delete operation (delete as in CRUD).
Expand Down Expand Up @@ -538,7 +543,7 @@ Fetcher.prototype['delete'] = function (resource, params, config, callback) {
return request
.params(params)
.clientConfig(config)
.end(callback)
.end(callback);
};

/**
Expand All @@ -559,7 +564,7 @@ Fetcher.prototype.updateOptions = function (options) {
*/
Fetcher.prototype.getServiceMeta = function () {
return this.serviceMeta;
}
};

module.exports = Fetcher;

Expand Down
12 changes: 12 additions & 0 deletions tests/unit/libs/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,9 @@ describe('Server Fetcher', function () {
it('should skip invalid GET resource', function (done) {
makeInvalidReqTest({method: 'GET', path: '/invalidService'}, 'Bad resource invalidService', done);
});
it('should sanitize resource name for invalid GET resource', function (done) {
makeInvalidReqTest({method: 'GET', path: '/invalid&Service'}, 'Bad resource invalid*Service', done);
});
it('should skip invalid POST request', function (done) {
makeInvalidReqTest({method: 'POST', body: {
requests: {
Expand All @@ -561,6 +564,15 @@ describe('Server Fetcher', function () {
}
}}, 'Bad resource invalidService', done);
});
it('should sanitize invalid POST request', function (done) {
makeInvalidReqTest({method: 'POST', body: {
requests: {
g0: {
resource: 'invalid&Service'
}
}
}}, 'Bad resource invalid*Service', done);
});
it('should skip POST request with empty req.body.requests object', function (done) {
makeInvalidReqTest({method: 'POST', body: { requests: {}}}, 'No resources', done);
});
Expand Down