Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage of Math.random() for the stream identifier #1057

Open
ngrigoriev opened this issue Oct 18, 2021 · 1 comment
Open

Usage of Math.random() for the stream identifier #1057

ngrigoriev opened this issue Oct 18, 2021 · 1 comment

Comments

@ngrigoriev
Copy link

ngrigoriev commented Oct 18, 2021

Versions of relevant software used

grpc-web 0.14.1

What happened

Possible unsafe use of Math.random() pseudorandom number generator.

This project uses Math.random() like this:

headerTrailerCombos((withHeaders, withTrailers) => {
--
226 |   |   | it(`should receive individual cadenced messages`, (done) => {
227 |   |   | let didGetOnHeaders = false;
228 |   |   | let onMessageId = 0;
229 |   |   |  
230 |   |   | const streamIdentifier = `rpc-${Math.random()}`;
231 |   |   |  
232 |   |   | const ping = new PingRequest();

What you expected to happen

Sonarqube recommends:

// === Client side ===
const crypto = window.crypto || window.msCrypto;
var array = new Uint32Array(1);
crypto.getRandomValues(array); // Compliant for security-sensitive use cases

// === Server side ===
const crypto = require('crypto');
const buf = crypto.randomBytes(1); // Compliant for security-sensitive use cases

How to reproduce it (as minimally and precisely as possible):

Run Sonarqube scan on the project

Full logs to relevant components

From Sonarqube output:

Using pseudorandom number generators (PRNGs) is security-sensitive. For example, it has led in the past to the following vulnerabilities:

CVE-2013-6386
CVE-2006-3419
CVE-2008-4102
When software generates predictable values in a context requiring unpredictability, it may be possible for an attacker to guess the next value that will be generated, and use this guess to impersonate another user or access sensitive information.

As the Math.random() function relies on a weak pseudorandom number generator, this function should not be used for security-critical applications or for protecting sensitive data. In such context, a cryptographically strong pseudorandom number generator (CSPRNG) should be used instead.

This way of generating the random numbers is flagged 4 times.

@johanbrandhorst
Copy link
Contributor

Thanks for your bug report. It sounds like this is only affecting the tests? I'm not sure we really care, but if you want to fix it feel free to open a PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants