Skip to content

Commit 33d58fb

Browse files
committed
Fix incorrect output issue with large numbers (and add a changelog). Closes #1.
1 parent aabdb1f commit 33d58fb

File tree

3 files changed

+65
-10
lines changed

3 files changed

+65
-10
lines changed

README.md

+7-1
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,10 @@ Optionally also accepts a nodeback as `cb`, but seriously, you should be using [
6060

6161
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"`.
6262

63-
The error message will provide more information, but this kind of error will generally mean that the arguments you've specified are somehow invalid.
63+
The error message will provide more information, but this kind of error will generally mean that the arguments you've specified are somehow invalid.
64+
65+
## Changelog
66+
67+
* __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 `>>`).
68+
* __1.0.1__ (March 8, 2016): Unimportant file cleanup.
69+
* __1.0.0__ (March 8, 2016): Initial release.

lib/index.js

+29-5
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,15 @@ function calculateParameters(range) {
3232

3333
bitsNeeded += 1;
3434
mask = mask << 1 | 1; /* 0x00001111 -> 0x00011111 */
35-
range = range >> 1; /* 0x01000000 -> 0x00100000 */
35+
36+
/* SECURITY PATCH (March 8, 2016):
37+
* As it turns out, `>>` is not the right operator to use here, and
38+
* using it would cause strange outputs, that wouldn't fall into
39+
* the specified range. This was remedied by switching to `>>>`
40+
* instead, and adding checks for input parameters being within the
41+
* range of 'safe integers' in JavaScript.
42+
*/
43+
range = range >>> 1; /* 0x01000000 -> 0x00100000 */
3644
}
3745

3846
return { bitsNeeded: bitsNeeded, bytesNeeded: bytesNeeded, mask: mask };
@@ -64,19 +72,35 @@ module.exports = function secureRandomNumber(minimum, maximum, cb) {
6472
throw new RandomGenerationError("The maximum value must be higher than the minimum value.");
6573
}
6674

75+
/* We hardcode the values for the following:
76+
*
77+
* https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER
78+
* https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
79+
*
80+
* ... as Babel does not appear to transpile them, despite being ES6.
81+
*/
82+
83+
if (minimum < -9007199254740991 || minimum > 9007199254740991) {
84+
throw new RandomGenerationError("The minimum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER.");
85+
}
86+
87+
if (maximum < -9007199254740991 || maximum > 9007199254740991) {
88+
throw new RandomGenerationError("The maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER.");
89+
}
90+
6791
var range = maximum - minimum;
6892

93+
if (range < -9007199254740991 || range > 9007199254740991) {
94+
throw new RandomGenerationError("The range between the minimum and maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER.");
95+
}
96+
6997
var _calculateParameters = calculateParameters(range);
7098

7199
var bitsNeeded = _calculateParameters.bitsNeeded;
72100
var bytesNeeded = _calculateParameters.bytesNeeded;
73101
var mask = _calculateParameters.mask;
74102

75103

76-
if (bitsNeeded > 53) {
77-
throw new RandomGenerationError("Cannot generate numbers larger than 53 bits.");
78-
}
79-
80104
return Promise.try(function () {
81105
return crypto.randomBytesAsync(bytesNeeded);
82106
}).then(function (randomBytes) {

src/index.js

+29-4
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,15 @@ function calculateParameters(range) {
3232

3333
bitsNeeded += 1;
3434
mask = mask << 1 | 1; /* 0x00001111 -> 0x00011111 */
35-
range = range >> 1; /* 0x01000000 -> 0x00100000 */
35+
36+
/* SECURITY PATCH (March 8, 2016):
37+
* As it turns out, `>>` is not the right operator to use here, and
38+
* using it would cause strange outputs, that wouldn't fall into
39+
* the specified range. This was remedied by switching to `>>>`
40+
* instead, and adding checks for input parameters being within the
41+
* range of 'safe integers' in JavaScript.
42+
*/
43+
range = range >>> 1; /* 0x01000000 -> 0x00100000 */
3644
}
3745

3846
return {bitsNeeded, bytesNeeded, mask};
@@ -64,13 +72,30 @@ module.exports = function secureRandomNumber(minimum, maximum, cb) {
6472
throw new RandomGenerationError("The maximum value must be higher than the minimum value.")
6573
}
6674

75+
/* We hardcode the values for the following:
76+
*
77+
* https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MIN_SAFE_INTEGER
78+
* https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
79+
*
80+
* ... as Babel does not appear to transpile them, despite being ES6.
81+
*/
82+
83+
if (minimum < -9007199254740991 || minimum > 9007199254740991) {
84+
throw new RandomGenerationError("The minimum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER.");
85+
}
86+
87+
if (maximum < -9007199254740991 || maximum > 9007199254740991) {
88+
throw new RandomGenerationError("The maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER.");
89+
}
90+
6791
let range = maximum - minimum;
68-
let {bitsNeeded, bytesNeeded, mask} = calculateParameters(range);
6992

70-
if (bitsNeeded > 53) {
71-
throw new RandomGenerationError("Cannot generate numbers larger than 53 bits.");
93+
if (range < -9007199254740991 || range > 9007199254740991) {
94+
throw new RandomGenerationError("The range between the minimum and maximum value must be inbetween MIN_SAFE_INTEGER and MAX_SAFE_INTEGER.");
7295
}
7396

97+
let {bitsNeeded, bytesNeeded, mask} = calculateParameters(range);
98+
7499
return Promise.try(() => {
75100
return crypto.randomBytesAsync(bytesNeeded);
76101
}).then((randomBytes) => {

0 commit comments

Comments
 (0)