Fix issue 323 - ESAPI Random Number Generation and CSRF Protection Broken

Problem was introduced in r722 - https://code.google.com/p/owasp-esapi-java/source/diff?spec=svn1942&r=722&format=side&path=/trunk/src/main/java/org/owasp/esapi/StringUtilities.java&old_path=/trunk/src/main/java/org/owasp/esapi/StringUtilities.java&old=561

Author: [email protected]

src/main/java/org/owasp/esapi/StringUtilities.java

@@ -61,12 +61,12 @@ public static String stripControls( String input ) {
     public static char[] union(char[]... list) {
     	StringBuilder sb = new StringBuilder();
 
-    	for (char[] characters : list) {
-	        for (int i = 0; i < list.length; i++) {
-	            if (!contains(sb, characters[i]))
-	                sb.append(list[i]);
-	        }
-    	}
+		for (char[] characters : list) {
+			for ( char c : characters ) {
+				if ( !contains( sb, c ) )
+					sb.append( c );
+			}
+		}
 
         char[] toReturn = new char[sb.length()];
         sb.getChars(0, sb.length(), toReturn, 0);

src/test/java/org/owasp/esapi/StringUtilitiesTest.java

@@ -1,5 +1,7 @@
 package org.owasp.esapi;
 
+import java.util.Arrays;
+
 import junit.framework.Test;
 import junit.framework.TestCase;
 import junit.framework.TestSuite;
@@ -42,7 +44,22 @@ public void testGetLevenshteinDistance() {
     		assertTrue( ex.getClass().getName().equals( IllegalArgumentException.class.getName() ));
     	}
     }
+
+    /** Test the union() method. */
+    public void testUnion() {
+		char[] a1 = { 'a', 'b', 'c' };
+		char[] a2 = { 'c', 'd', 'e' };
+		char[] union = StringUtilities.union(a1, a2);
+		assertTrue( Arrays.equals( union, new char[] {'a','b','c','d','e' } ) );
+    }
 
+    /** Test the contains() method. */
+    public void contains() {
+		StringBuilder sb = new StringBuilder( "abc" );
+		assertTrue( StringUtilities.contains(sb, 'b') );
+		assertFalse( StringUtilities.contains(sb, 'x') );
+    }
+
     /** Test the notNullOrEmpty() method. */
     public void testNotNullOrEmpty() {
     	String str = "A string";

src/test/java/org/owasp/esapi/reference/RandomizerTest.java

@@ -15,6 +15,8 @@
  */
 package org.owasp.esapi.reference;
 
+import java.io.FileWriter;
+import java.io.IOException;
 import java.util.ArrayList;
 
 import junit.framework.Test;
@@ -76,16 +78,28 @@ public static Test suite() {
     public void testGetRandomString() {
         System.out.println("getRandomString");
         int length = 20;
+        int trials = 1000;
         Randomizer instance = ESAPI.randomizer();
-        for ( int i = 0; i < 100; i++ ) {
+        int[] counts = new int[128];
+        for ( int i = 0; i < 1000; i++ ) {
             String result = instance.getRandomString(length, EncoderConstants.CHAR_ALPHANUMERICS );
             for ( int j=0;j<result.length();j++ ) {
-            	if ( !Codec.containsCharacter( result.charAt(j), EncoderConstants.CHAR_ALPHANUMERICS) ) {
-            		fail();
-            	}
+            	char c = result.charAt(j);
+            	counts[c]++;
             }
             assertEquals(length, result.length());
         }
+        
+        // Simple check to see if the overall character counts are within 10% of each other
+        int min=Integer.MAX_VALUE;
+        int max=0;
+        for ( int i = 0; i < 128; i++ ) {
+        	if ( counts[i] > max ) { max = counts[i]; } 
+        	if ( counts[i] > 0 && counts[i] < min ) { min = counts[i]; }
+        	if ( max - min > trials/10 ) {
+        		fail( "getRandomString randomness counts are off" );
+        	}
+        }
     }
 
     /**
@@ -140,5 +154,24 @@ public void testGetRandomGUID() throws EncryptionException {
         }
     }
 
+    
+    /**
+     * Run this class to generate a file named "tokens.txt" with 20,000 random 20 character ALPHANUMERIC tokens.
+     * Use Burp Pro sequencer to load this file and run a series of randomness tests.
+     * 
+     * NOTE: be careful not to include any CRLF characters (10 or 13 ASCII) because they'll create new tokens
+     * Check to be sure your analysis tool loads exactly 20,000 tokens of 20 characters each.
+     */
+    
+	public static void main(String[] args) throws IOException {
+		FileWriter fw = new FileWriter("tokens.txt");
+		for (int i = 0; i < 20000; i++) {
+			String token = ESAPI.randomizer().getRandomString(20, EncoderConstants.CHAR_ALPHANUMERICS);
+			fw.write(token + "\n");
+		}
+		fw.close();
+	}
+
+
 
 }