Comment by chrismorgan
1 day ago
Admittedly I have done some cryptographic string generation based on different alphabet sizes and characteristics a few years ago, which is pretty specifically relevant, and I’m competent at cryptographic and security concerns for a layman, but I certainly hope security reviewers will be more skilled at these things than me.
I’m very confident I would have noticed this bias in a first pass of reviewing the code. The very first thing you do in a security review is look at where you use `crypto`, what its inputs are, and what you do with its outputs, very carefully. On seeing that %, I would have checked characters.length and found it to be 62, not a factor of 256; so you need to mess around with base conversion, or change the alphabet, or some other such trick.
This bothers me and makes me lose confidence in the review performed.
But... is it a real problem? As the author says, the entropy reduction is tiny.
It shows carelessness or incompetence or a combination thereof which extend to the entire code base.