From 33d58fb71200a4d613fbd7bd2c73e82865d5fa5d Mon Sep 17 00:00:00 2001 From: Sven Slootweg Date: Tue, 8 Mar 2016 23:10:35 +0100 Subject: [PATCH] Fix incorrect output issue with large numbers (and add a changelog). Closes #1. --- README.md | 8 +++++++- lib/index.js | 34 +++++++++++++++++++++++++++++----- src/index.js | 33 +++++++++++++++++++++++++++++---- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 7ea031f..17d87f2 100644 --- a/README.md +++ b/README.md @@ -60,4 +60,10 @@ Optionally also accepts a nodeback as `cb`, but seriously, you should be using [ Any errors that occur during the random number generation process will be of this type. The error object will also have a `code` property, set to the string `"RandomGenerationError"`. -The error message will provide more information, but this kind of error will generally mean that the arguments you've specified are somehow invalid. \ No newline at end of file +The error message will provide more information, but this kind of error will generally mean that the arguments you've specified are somehow invalid. + +## Changelog + +* __1.0.2__ (March 8, 2016): __*Security release!*__ Patched handling of large numbers; input values are now checked for `MIN_SAFE_INTEGER` and `MAX_SAFE_INTEGER`, and the correct bitwise operator is used (`>>>` rather than `>>`). +* __1.0.1__ (March 8, 2016): Unimportant file cleanup. +* __1.0.0__ (March 8, 2016): Initial release. \ No newline at end of file diff --git a/lib/index.js b/lib/index.js index e1f35e3..96ca40d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -32,7 +32,15 @@ function calculateParameters(range) { bitsNeeded += 1; mask = mask << 1 | 1; /* 0x00001111 -> 0x00011111 */ - range = range >> 1; /* 0x01000000 -> 0x00100000 */ + + /* SECURITY PATCH (March 8, 2016): + * As it turns out, `>>` is not the right operator to use here, and + * using it would cause strange outputs, that wouldn't fall into + * the specified range. This was remedied by switching to `>>>` + * instead, and adding checks for input parameters being within the + * range of 'safe integers' in JavaScript. + */ + range = range >>> 1; /* 0x01000000 -> 0x00100000 */ } return { bitsNeeded: bitsNeeded, bytesNeeded: bytesNeeded, mask: mask }; @@ -64,8 +72,28 @@ module.exports = function secureRandomNumber(minimum, maximum, cb) { throw new RandomGenerationError("The maximum value must be higher than the minimum value."); } + /* We hardcode the values for the following: + * + * https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER + * https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER + * + * ... as Babel does not appear to transpile them, despite being ES6. + */ + + if (minimum < -9007199254740991 || minimum > 9007199254740991) { + throw new RandomGenerationError("The minimum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER."); + } + + if (maximum < -9007199254740991 || maximum > 9007199254740991) { + throw new RandomGenerationError("The maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER."); + } + var range = maximum - minimum; + if (range < -9007199254740991 || range > 9007199254740991) { + throw new RandomGenerationError("The range between the minimum and maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER."); + } + var _calculateParameters = calculateParameters(range); var bitsNeeded = _calculateParameters.bitsNeeded; @@ -73,10 +101,6 @@ module.exports = function secureRandomNumber(minimum, maximum, cb) { var mask = _calculateParameters.mask; - if (bitsNeeded > 53) { - throw new RandomGenerationError("Cannot generate numbers larger than 53 bits."); - } - return Promise.try(function () { return crypto.randomBytesAsync(bytesNeeded); }).then(function (randomBytes) { diff --git a/src/index.js b/src/index.js index a9fb872..900aae3 100644 --- a/src/index.js +++ b/src/index.js @@ -32,7 +32,15 @@ function calculateParameters(range) { bitsNeeded += 1; mask = mask << 1 | 1; /* 0x00001111 -> 0x00011111 */ - range = range >> 1; /* 0x01000000 -> 0x00100000 */ + + /* SECURITY PATCH (March 8, 2016): + * As it turns out, `>>` is not the right operator to use here, and + * using it would cause strange outputs, that wouldn't fall into + * the specified range. This was remedied by switching to `>>>` + * instead, and adding checks for input parameters being within the + * range of 'safe integers' in JavaScript. + */ + range = range >>> 1; /* 0x01000000 -> 0x00100000 */ } return {bitsNeeded, bytesNeeded, mask}; @@ -64,13 +72,30 @@ module.exports = function secureRandomNumber(minimum, maximum, cb) { throw new RandomGenerationError("The maximum value must be higher than the minimum value.") } + /* We hardcode the values for the following: + * + * https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER + * https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER + * + * ... as Babel does not appear to transpile them, despite being ES6. + */ + + if (minimum < -9007199254740991 || minimum > 9007199254740991) { + throw new RandomGenerationError("The minimum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER."); + } + + if (maximum < -9007199254740991 || maximum > 9007199254740991) { + throw new RandomGenerationError("The maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER."); + } + let range = maximum - minimum; - let {bitsNeeded, bytesNeeded, mask} = calculateParameters(range); - if (bitsNeeded > 53) { - throw new RandomGenerationError("Cannot generate numbers larger than 53 bits."); + if (range < -9007199254740991 || range > 9007199254740991) { + throw new RandomGenerationError("The range between the minimum and maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER."); } + let {bitsNeeded, bytesNeeded, mask} = calculateParameters(range); + return Promise.try(() => { return crypto.randomBytesAsync(bytesNeeded); }).then((randomBytes) => {