Skip to content

Commit

Permalink
sanitize requests in the forms controller. Issue #1844
Browse files Browse the repository at this point in the history
Added a `db.sanitizeResponse` that can be called from any controller.  The only
problematic function right now is `forms.getForm` because it returned headers
coming from nano.  Sanitized those headers by removing `uri` and `statusCode`
and wrote tests around it.  This could potentially leak auth information to the
client.  See https://github.com/dscape/nano/blob/master/lib/nano.js#L195

TODO: open issue in nano project and patch
  • Loading branch information
Milan Andric committed Feb 3, 2016
1 parent 5ece6e2 commit beb2a7c
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 3 deletions.
6 changes: 3 additions & 3 deletions controllers/forms.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function listFormsXML(data, template, callback) {
'Content-Type': 'text/xml; charset=utf-8',
'X-OpenRosa-Version': '1.0'
};
callback(null, xml, headers);
db.sanitizeResponse(null, xml, headers, callback);
});
}

Expand All @@ -80,7 +80,7 @@ function listForms(data, callback) {
var headers = {
'Content-Type': 'application/json; charset=utf-8'
};
callback(null, JSON.stringify(ret), headers);
db.sanitizeResponse(null, JSON.stringify(ret), headers, callback);
}

module.exports = {
Expand Down Expand Up @@ -125,7 +125,7 @@ module.exports = {
//'Content-Type': 'text/xml; charset=utf-8',
//'Content-Disposition': 'attachment; filename="{{file}}";'
// .replace('{{file}}', req.params.form),
callback(null, body, headers);
db.sanitizeResponse(null, body, headers, callback);
});
});
}
Expand Down
18 changes: 18 additions & 0 deletions db-nano.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,21 @@ var path = require('path'),
nano = require('nano');

var couchUrl = process.env.COUCH_URL;

var sanitizeResponse = function(err, body, headers, callback) {
// Remove the `uri` and `statusCode` headers passed in from nano. This
// could potentially leak auth information to the client. See
// https://github.com/dscape/nano/blob/master/lib/nano.js#L195
// TODO: open issue in nano project and patch
var denyHeaders = ['uri', 'statuscode'];
for (var k in headers) {
if (denyHeaders.indexOf(k.toLowerCase()) >= 0) {
delete headers[k];
}
}
callback(err, body, headers);
};

if (couchUrl) {
var parsedUrl = url.parse(couchUrl);

Expand Down Expand Up @@ -57,15 +72,18 @@ if (couchUrl) {
module.exports.settings.ddoc);
module.exports.request({ path: uri }, cb);
};
module.exports.sanitizeResponse = sanitizeResponse;
} else if (process.env.TEST_ENV) {
// Running tests only
module.exports = {
fti: function() {},
request: function() {},
getPath: function() {},
settings: {},
sanitizeResponse: sanitizeResponse,
medic: {
view: function() {},
attachment: {},
get: function() {},
insert: function() {},
updateWithHandler: function() {}
Expand Down
117 changes: 117 additions & 0 deletions tests/forms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
var controller = require('../controllers/forms'),
db = require('../db-nano'),
sinon = require('sinon');

exports.tearDown = function (callback) {
if (db.request.restore) {
db.request.restore();
}
if (db.sanitizeResponse.restore) {
db.sanitizeResponse.restore();
}
if (db.medic.view.restore) {
db.medic.view.restore();
}
if (db.medic.attachment.get.restore) {
db.medic.attachment.get.restore();
}
callback();
};

exports['getForm returns error from view query'] = function(test) {
test.expect(2);
var req = sinon.stub(db.medic, 'view').callsArgWith(3, 'icky');
controller.getForm('', '', function(err, results) {
test.equals(err, 'icky');
test.equals(req.callCount, 1);
test.done();
});
};

exports['getForm returns form not found message on empty view query'] = function(test) {
test.expect(2);
var req = sinon.stub(db.medic, 'view').callsArgWith(3, null, {rows: []});
controller.getForm('', '', function(err, body, headers) {
test.equals(err.message, 'Form not found.');
test.equals(req.callCount, 1);
test.done();
});
};

exports['getForm returns error from attachment query'] = function(test) {
test.expect(3);
var req1 = sinon.stub(db.medic, 'view').callsArgWith(3, null, {rows: [1]});
var req2 = sinon.stub(db.medic.attachment, 'get').callsArgWith(2, 'boop');
controller.getForm('', '', function(err, body, headers) {
test.equals(err, 'boop');
test.equals(req1.callCount, 1);
test.equals(req2.callCount, 1);
test.done();
});
};

exports['getForm returns body and headers from attachment query'] = function(test) {
test.expect(2);
sinon.stub(db.medic, 'view').callsArgWith(3, null, {rows: [1]});
sinon.stub(db.medic.attachment, 'get').callsArgWith(2, null, 'foo', {
'content-type': 'xml'
});
controller.getForm('', '', function(err, body, headers) {
test.equals(body, 'foo');
test.deepEqual(headers, {'content-type': 'xml'});
test.done();
});
};

exports['getForm sanitizes bad headers from attachment query'] = function(test) {
test.expect(3);
sinon.stub(db.medic, 'view').callsArgWith(3, null, {rows: [1]});
sinon.stub(db.medic.attachment, 'get').callsArgWith(2, null, 'foo', {
'content-type': 'xml',
'uri' : 'http://admin:secret@localhost',
'statusCode' : 'junk'
});
var spy = sinon.spy(db, 'sanitizeResponse');
controller.getForm('', '', function(err, body, headers) {
test.equals(body, 'foo');
test.deepEqual(headers, {'content-type': 'xml'});
test.ok(spy.called);
test.done();
});
};

exports['listForms returns error from view query'] = function(test) {
test.expect(2);
var req = sinon.stub(db.medic, 'view').callsArgWith(3, 'icky');
controller.listForms({}, function(err, body, headers) {
test.equals(err, 'icky');
test.equals(req.callCount, 1);
test.done();
});
};

exports['listForms sanitizes response'] = function(test) {
test.expect(2);
sinon.stub(db.medic, 'view').callsArgWith(3, null, {
rows: [1]
});
var spy = sinon.spy(db, 'sanitizeResponse');
controller.listForms({}, function(err, body, headers) {
test.equals(err, null);
test.equals(spy.callCount, 1);
test.done();
});
};

exports['listForms sanitizes openrosa response'] = function(test) {
test.expect(2);
sinon.stub(db.medic, 'view').callsArgWith(3, null, {
rows: [1]
});
var spy = sinon.spy(db, 'sanitizeResponse');
controller.listForms({'x-openrosa-version': '1.0'}, function(err, body, headers) {
test.equals(err, null);
test.equals(spy.callCount, 1);
test.done();
});
};

0 comments on commit beb2a7c

Please sign in to comment.