antlr / antlr4

ANTLR (ANother Tool for Language Recognition) is a powerful parser generator for reading, processing, executing, or translating structured text or binary files.
http://antlr.org
BSD 3-Clause "New" or "Revised" License
16.97k stars 3.26k forks source link

Math.random raising security flags in White Hat. #2261

Open amodlagu opened 6 years ago

amodlagu commented 6 years ago

Target: Javascript, version: 4.7

Security vulnerability detected in Javascript library.

In Utils.js, String.prototype.seed = String.prototype.seed || Math.round(Math.random() * Math.pow(2, 32));

We are including the Antlr library to support an online IDE for the formula language in our product. Security team using sentinel WhiteHat reported that Math.random() is not allowed. Would it be possible to replace Math.random with a cryptographically secure random number generator. In the browser, there are window.crypto or window.msCrypto (IE 11). But there could be other crypto libraries as well.

We are planning to make a similar change in the node modules that we are including, but could a fix be provided in the Antlr library as well?

piotrl commented 5 years ago

I would add, that overriding String.prototype.seed and String.prototype.hashCode is affecting global scope of application, which is also not safe or secure, especially when bundled with different libraries - we might never know if someone else will override it (especially hashCode)

ericvergnaud commented 5 years ago

@piotrl strictly speaking this is an extension not an override. The risk you mention is easy to avoid using a try/finally in your code

ericvergnaud commented 5 years ago

@amodlagu you might want to let your WhiteHat team know that Math.random is perfectly secure for non cryptographic usage, so they loosen up their rules