Skip to content

Commit d811518

Browse files
committed
fixed from doc review, and add fixed example for js/biased-cryptographic-random using a secure library
1 parent 6968796 commit d811518

3 files changed

Lines changed: 24 additions & 16 deletions

File tree

javascript/ql/src/Security/CWE-327/BadRandomness.qhelp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
<qhelp>
55
<overview>
66
<p>
7-
Generating secure random numbers can be an important part of creating
8-
a secure software system, and for that purpose there exists secure APIs
9-
for creating cryptographically secure random numbers.
7+
Generating secure random numbers can be an important part of creating a
8+
secure software system. This can be done using APIs that create
9+
cryptographically secure random numbers.
1010
</p>
1111
<p>
1212
However, using some mathematical operations on these cryptographically
@@ -28,7 +28,7 @@
2828
</recommendation>
2929
<example>
3030
<p>
31-
The below example uses the modulo operator to create an array of 10 random digits
31+
The example below uses the modulo operator to create an array of 10 random digits
3232
using random bytes as the source for randomness.
3333
</p>
3434
<sample src="examples/bad-random.js" />
@@ -38,12 +38,17 @@
3838
between 6 and 9.
3939
</p>
4040
<p>
41-
The issue has been fixed in the code below, where the random byte is discarded if
42-
the value was greater than or equal to 250.
41+
The issue has been fixed in the code below by using a library that correctly generates
42+
cryptographically secure random values.
43+
</p>
44+
<sample src="examples/bad-random-fixed.js" />
45+
<p>
46+
Alternatively, the issue can be fixed by fixing the math in the original code.
47+
In the code below the random byte is discarded if the value is greater than or equal to 250.
4348
Thus the modulo operator is used on a uniformly random number between 0 and 249, which
4449
results in a uniformly random digit between 0 and 9.
4550
</p>
46-
<sample src="examples/bad-random-fixed.js" />
51+
<sample src="examples/bad-random-fixed2.js" />
4752

4853
</example>
4954

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,3 @@
1-
const crypto = require('crypto');
1+
const cryptoRandomString = require('crypto-random-string');
22

3-
const digits = [];
4-
while (digits.length < 10) {
5-
const byte = crypto.randomBytes(1)[0];
6-
if (byte >= 250) {
7-
continue;
8-
}
9-
digits.push(byte % 10); // OK
10-
}
3+
const digits = cryptoRandomString({length: 10, type: 'numeric'});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
const crypto = require('crypto');
2+
3+
const digits = [];
4+
while (digits.length < 10) {
5+
const byte = crypto.randomBytes(1)[0];
6+
if (byte >= 250) {
7+
continue;
8+
}
9+
digits.push(byte % 10); // OK
10+
}

0 commit comments

Comments
 (0)