Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request #16 from spumko/security-updates
Security update
  • Loading branch information
stongo committed Jul 31, 2014
2 parents 9c7afbb + 1a8e694 commit 5e6d4f5
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 15 deletions.
37 changes: 37 additions & 0 deletions SECURITY.md
@@ -0,0 +1,37 @@
# Reporting a security bug
All security bugs are taken seriously and should be reported by email to stongo@gmail.com.

Your email will be acknowledged within 24 hours, and you’ll receive a more detailed response to your email within 48 hours indicating the next steps in handling your report.

After the initial reply to your report, the security team will endeavor to keep you informed of the progress being made towards a fix and full announcement, and may ask for additional information or guidance surrounding the reported issue. These updates will be sent at least every five days, in practice, this is more likely to be every 24-48 hours.

If you have not received a reply to your email within 48 hours, or have not heard from the security team for the past five days, there are a few steps you can take:

- Email [Marcus Stong](mailto:stongo@gmail.com)
- Give the Spumko team a heads up on IRC in #hapi on irc.freenode.net

Thank you for taking the time to disclose the issue to us. Your efforts and responsible disclosure are greatly appreciated!

# Disclosure Policy

The security report is received and is assigned a primary handler. This person will coordinate the fix and release process.

The problem is confirmed. Code is audited to find any potential similar problems.

Fixes are prepared. These fixes are not committed to the public repository but rather held locally pending the announcement.

This process can take some time, especially when coordination is required with maintainers of other projects. Every effort will be made to handle the bug in as timely a manner as possible.

A blog post about the issue will be published, customers notified and attribution to the finder given.

# Comments on this Policy
Can you help make this policy / process better? Please email stongo@gmail.com with your comments.

# History

* When CORS set to true and Origin request header exists, it could lead to a crumb cookie being set on a different domain. Fixed in version 3.0.0. Attributed to Marcus Stong and Tom Steele.

# Credits

Thanks to Adam Baldwin for explaining the importance of a SECURITY.md file:
https://blog.liftsecurity.io/2013/06/02/security-md-improved-security-disclosure
2 changes: 1 addition & 1 deletion example/restful.js
Expand Up @@ -4,7 +4,7 @@ var server = Hapi.createServer('127.0.0.1', 8000);

// Add Crumb plugin

server.pack.require('../', { restful: true }, function(err) {
server.pack.register({ plugin: require('../'), options: { restful: true } }, function(err) {
if (err) throw err;
});

Expand Down
4 changes: 2 additions & 2 deletions example/server.js
Expand Up @@ -4,14 +4,14 @@ var serverOptions = {
views: {
path: __dirname + '/templates',
engines: {
html: 'handlebars'
html: require('handlebars')
}
}
};

var server = new Hapi.Server('127.0.0.1', 8000, serverOptions);

server.pack.require('../', { cookieOptions: { isSecure: false } }, function (err) {
server.pack.register({ plugin: require('../'), options: { cookieOptions: { isSecure: false } } }, function (err) {
if (err) throw err;
});

Expand Down
5 changes: 3 additions & 2 deletions lib/index.js
Expand Up @@ -54,8 +54,9 @@ exports.register = function (plugin, options, next) {

// Set crumb cookie and calculate crumb

if (settings.autoGenerate ||
request.route.plugins._crumb) {
if ((settings.autoGenerate ||
request.route.plugins._crumb) &&
!request.headers.origin) {

generate(request, reply);
}
Expand Down
4 changes: 2 additions & 2 deletions package.json
@@ -1,7 +1,7 @@
{
"name": "crumb",
"description": "CSRF crumb generation and validation plugin",
"version": "2.2.0",
"version": "3.0.0",
"author": "Eran Hammer <eran@hueniverse.com> (http://hueniverse.com)",
"contributors": [
"Marcus Stong <stongo@gmail.com>",
Expand Down Expand Up @@ -30,7 +30,7 @@
"hapi": ">=2.x.x"
},
"devDependencies": {
"hapi": "5.x.x",
"hapi": "6.x.x",
"handlebars": "1.3.x",
"lab": "3.x.x"
},
Expand Down
52 changes: 46 additions & 6 deletions test/index.js
Expand Up @@ -27,7 +27,7 @@ describe('Crumb', function () {
views: {
path: __dirname + '/templates',
engines: {
html: 'handlebars'
html: require('handlebars')
}
}
};
Expand Down Expand Up @@ -81,10 +81,16 @@ describe('Crumb', function () {

return reply.view('index');
}
},
{
method: 'GET', path: '/7', handler: function (request, reply) {

return reply(null).redirect('/1');
}
}
]);

server1.pack.require('../', { cookieOptions: { isSecure: true } }, function (err) {
server1.pack.register({ plugin: require('../'), options: { cookieOptions: { isSecure: true } } }, function (err) {

expect(err).to.not.exist;
server1.inject({ method: 'GET', url: '/1' }, function (res) {
Expand Down Expand Up @@ -146,9 +152,16 @@ describe('Crumb', function () {
var cookie = header[0].match(/crumb=([^\x00-\x20\"\,\;\\\x7F]*)/);
expect(res.result).to.equal('<!DOCTYPE html><html><head><title></title></head><body><div><h1></h1><h2>' + cookie[1] + '</h2></div></body></html>');

done();
});
});

server1.inject({method: 'GET', url: '/7'}, function(res) {

var cookie = res.headers['set-cookie'].toString();
expect(cookie).to.contain('crumb');

done();
});
});
});
});

This comment has been minimized.

Copy link
@Kaylakuntz

Kaylakuntz Oct 23, 2014

uLeave a line note

Expand All @@ -173,7 +186,7 @@ describe('Crumb', function () {
}
});

server2.pack.require('../', { cookieOptions: { isSecure: true }, addToViewContext: false }, function (err) {
server2.pack.register({ plugin: require('../'), options: { cookieOptions: { isSecure: true }, addToViewContext: false } }, function (err) {

expect(err).to.not.exist;
server2.inject({ method: 'GET', url: '/1' }, function (res) {
Expand All @@ -200,7 +213,7 @@ describe('Crumb', function () {
}
});

server3.pack.require('../', null, function (err) {
server3.pack.register({ plugin: require('../'), options: null }, function (err) {

expect(err).to.not.exist;

Expand Down Expand Up @@ -241,7 +254,7 @@ describe('Crumb', function () {
}
]);

server3.pack.require('../', { autoGenerate: false }, function (err) {
server3.pack.register({ plugin: require('../'), options: { autoGenerate: false } }, function (err) {

expect(err).to.not.exist;

Expand All @@ -258,4 +271,31 @@ describe('Crumb', function () {
});
});
});

it('does not set crumb cookie insecurely', function(done) {
var options = {
cors: true
}
var server4 = new Hapi.Server(options);
server4.route([
{
method: 'GET', path: '/1', handler: function (request, reply) {

return reply('test');
}
}
]);
server4.pack.register({ plugin: require('../'), options: null }, function (err) {
expect(err).to.not.exist;
var headers = {};
headers['Origin'] = '127.0.0.1'
server4.inject({ method: 'GET', url: '/1', headers: headers }, function (res) {

var header = res.headers['set-cookie'];
expect(header).to.not.contain('crumb');

done();
});
});
});
});
4 changes: 2 additions & 2 deletions test/restful.js
Expand Up @@ -27,7 +27,7 @@ describe('Crumb', function () {
views: {
path: __dirname + '/templates',
engines: {
html: 'handlebars'
html: require('handlebars')
}
}
};
Expand Down Expand Up @@ -97,7 +97,7 @@ describe('Crumb', function () {

]);

server.pack.require('../', { restful: true, cookieOptions: { isSecure: true } }, function (err) {
server.pack.register({plugin: require('../'), options: { restful: true, cookieOptions: { isSecure: true } } }, function (err) {

expect(err).to.not.exist;
server.inject({ method: 'GET', url: '/1' }, function (res) {
Expand Down

0 comments on commit 5e6d4f5

Please sign in to comment.