[Esapi-user] ESAPI Random Number Generation Broken

Kevin W. Wall kevin.w.wall at gmail.com
Wed Apr 9 04:17:48 UTC 2014


On Tue, Apr 8, 2014 at 1:37 AM, Jim Manico <jim.manico at owasp.org> wrote:
> Ok, first of all this is (another) clear proof of Rooks original
> report. This is a serious problem. I suggest you create a second
> implementation of this (fork) and change the default config to point
> to it. That way the more secure option is default, and those who need
> backwards compatibility have an option to roll back to. Just an idea,
> I'm not 100% clear on this.

That's probably a good idea, but to do this, we are also going to have to
fix the deployment issues with not getting the official ESAPI.properties
deployed with the new deployments. I don't know how to do that. It's
either a Maven issue or an issue with Maven Central.
(See Google issue 309,
https://code.google.com/p/owasp-esapi-java/issues/detail?id=309)

Also, regarding the CSRF token.  I don't care *how* well you pick 8
random characters, if you restrict that character set to only 62 characters,
there will not be nearly enough entropy to provide security. Presumably
you want to have those CSRF tokens be cryptographically secure.
The NIST minimum recommendation for this would be around 80 bits
or so.  Even if you used a full 8 bytes that's only 64 bits. If you then
take a reduced character set you further reduce that. It you had (say)
64 characters in the character set, you have 64 of a possible 256 bit
patterns per byte, so you are only using 25% of the possible bit
pattern space. So that 64 bits turns into 16 bits of entropy. That's just
too small. To get the minimum recommended # of bits (80), you
would need 20 characters, not just 8, assuming that you only use
64 different characters in your character set (e.g., all the alphanumeric
plus '_' and say '.').  Make sense?  By mapping things down to
64 (or 62 chars, which is what ESAPI is currently doing), you need
a lot more characters than you think...about 4 times as many
if my math is correct.  BTW, I figured out a way to do that without
using the potentially biased [Secure]Random.nextInt(int) call. It's just:

    byte[] nonce = new byte[ n ];
    secureRandom.nextBytes(nonce);
    StringBuilder result = new StringBuilder(length);
    for( int i = 0; i < nonce.length; i++ ) {
        char c = characterSet[ Math.abs( (nonce[i] % characterSet.length) ) ];
        result.append( c );
    }
    return result.toString();

I added two characters ('_' and '.', but any additional 2 unreserved
characters from RFC 3986 - Section 2.3 would do) to the 62 characters
(A-Z, a-z, and 0-9) that ESAPI was using just to get rid of potential bias
(since 64 divides 256).  I tested the above with 8 characters in
Burp, and it reported 44 bits of entropy. I therefore conclude that
the Burp tests are at best flaky. Either that or something is wrong
with my analysis and arithmetic, but like I said, it should be
around 16 bits, not 44 bits. So either Burp's sequencer is
flaky or we are not correctly understanding it's output. Anyone
know?

Lastly, any official fixes to ESAPI is going to have to wait until
after I finish my taxes (which likely won't be until 4/15) unless
someone else wants to work on this.

> Can you also update
> https://code.google.com/p/owasp-esapi-java/issues/detail?id=323 label
> it critical or whatever you think is appropriate?

Done.

-kevin
-- 
Blog: http://off-the-wall-security.blogspot.com/
NSA: All your crypto bit are belong to us.


More information about the Esapi-user mailing list