Skip to content

Commit

Permalink
Verification with an asymmetric key of a token signed with a symmetri…
Browse files Browse the repository at this point in the history
…c key

There is a vulnerability in this module when the verification part is expecting a token digitally signed with an asymetric key (RS/ES family) of algorithms but instead the attacker send a token digitally signed with a symmetric algorithm (HS* family).

The issue is because this library has the very same signature to verify both type of tokens (parameter: `secretOrPublicKey`).

This change adds a new parameter to the verify called `algorithms`. This can be used to specify a list of supported algorithms, but the default value depends on the secret used: if the secretOrPublicKey contains the string `BEGIN CERTIFICATE` the default is `[ 'RS256','RS384','RS512','ES256','ES384','ES512' ]` otherwise is `[ 'HS256','HS384','HS512' ]`.
  • Loading branch information
jfromaniello committed Mar 16, 2015
1 parent f9f3c34 commit 1bb584b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
3 changes: 2 additions & 1 deletion .jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"it": true,
"require": true,
"atob": false,
"escape": true
"escape": true,
"before": true
}
}
11 changes: 11 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ module.exports.verify = function(jwtString, secretOrPublicKey, options, callback
return done(new JsonWebTokenError('jwt signature is required'));
}

if (!options.algorithms) {
options.algorithms = ~secretOrPublicKey.toString().indexOf('BEGIN CERTIFICATE') ?
[ 'RS256','RS384','RS512','ES256','ES384','ES512' ] :
[ 'HS256','HS384','HS512' ];
}

var valid;

try {
Expand All @@ -126,6 +132,11 @@ module.exports.verify = function(jwtString, secretOrPublicKey, options, callback
return done(err);
}

var header = jws.decode(jwtString).header;
if (!~options.algorithms.indexOf(header.alg)) {
return done(new JsonWebTokenError('invalid signature'));
}

if (typeof payload.exp !== 'undefined' && !options.ignoreExpiration) {
if (typeof payload.exp !== 'number') {
return done(new JsonWebTokenError('invalid exp value'));
Expand Down
20 changes: 20 additions & 0 deletions test/wrong_alg.tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
var fs = require('fs');
var path = require('path');
var jwt = require('../index');
var JsonWebTokenError = require('../lib/JsonWebTokenError');
var expect = require('chai').expect;


var pub = fs.readFileSync(path.join(__dirname, 'pub.pem'), 'utf8');
// priv is never used
// var priv = fs.readFileSync(path.join(__dirname, 'priv.pem'));

var TOKEN = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJmb28iOiJiYXIiLCJpYXQiOjE0MjY1NDY5MTl9.ETgkTn8BaxIX4YqvUWVFPmum3moNZ7oARZtSBXb_vP4';

describe('signing with pub key as symmetric', function () {
it('should not verify', function () {
expect(function () {
jwt.verify(TOKEN, pub);
}).to.throw(JsonWebTokenError, /invalid signature/);
});
});

0 comments on commit 1bb584b

Please sign in to comment.