search

mebigfatguy / fb-contrib

a FindBugs/SpotBugs plugin for doing static code analysis for java code bases
http://fb-contrib.sf.net
GNU Lesser General Public License v2.1
151 stars 45 forks source link

SecureRandom and Random (MDM_SECURERANDOM & MDM_SECURERANDOM) #36

Open h3xstream opened 9 years ago

h3xstream commented 9 years ago

I think two detectors related to Randomness are incorrect.

MDM_SECURERANDOM

The SecureRandom() constructors and SecureRandom.getSeed() method are deprecated. Call SecureRandom.getInstance() and SecureRandom.getInstance().generateSeed() instead.

The constructor of SecureRandom doesn't appears to be deprecated according to the javadoc : http://docs.oracle.com/javase/8/docs/api/java/security/SecureRandom.html

MDM_RANDOM_SEED

Random() constructor without a seed is insecure because it defaults to easily guessable seed: System.currentTimeMillis(). Initialize seed with Random(SecureRandom.getInstance().generateSeed()) or use SecureRandom instead.

The first recommendation to give a good seed is a bad idea. The initial state will be unpredictable but given a single long value it is possible to recover all the following values. This is not an obvious behavior. It can introduce vulnerability like CVE-2014-7809 or CVE-2015-0201.

mebigfatguy commented 9 years ago

These patterns should only fire for code compiled against jdk 1.5, which while they are not deprecated are recommended against in the javadoc. I suppose we could just remove them at this point. I didn't write these originally. I don't recall who submitted them, I'd have to dig thru the sourceforge history to find out.