Verification with an asymmetric key of a token signed with a symmetric key

jfromaniello · jfromaniello · commit 1bb584bc3822 · 2015-03-16T20:36:22.000-03:00
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' ]`.
diff --git a/.jshintrc b/.jshintrc
@@ -15,6 +15,7 @@
     "it": true,
     "require": true,
     "atob": false,
-    "escape": true
+    "escape": true,
+    "before": true
   }
 }
diff --git a/index.js b/index.js
@@ -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 {
@@ -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'));
diff --git a/test/wrong_alg.tests.js b/test/wrong_alg.tests.js
@@ -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/);
+  });
+});

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@`
`15`	`15`	`"it": true,`
`16`	`16`	`"require": true,`
`17`	`17`	`"atob": false,`
`18`		`- "escape": true`
	`18`	`+ "escape": true,`
	`19`	`+ "before": true`
`19`	`20`	`}`
`20`	`21`	`}`