Use secure randomness in Arr:random and Arr:shuffle

laravel · Feb 14, 2023 · 6e7534e · 6e7534e
1 parent fcb6558
commit 6e7534e
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 21 deletions.
diff --git a/src/Illuminate/Collections/Arr.php b/src/Illuminate/Collections/Arr.php
@@ -635,28 +635,15 @@ public static function random($array, $number = null, $preserveKeys = false)
         }
 
         if (is_null($number)) {
-            return $array[array_rand($array)];
+            $values = array_values($array);
+            return $values[random_int(0, $count - 1)];
         }
 
         if ((int) $number === 0) {
             return [];
         }
 
-        $keys = array_rand($array, $number);
-
-        $results = [];
-
-        if ($preserveKeys) {
-            foreach ((array) $keys as $key) {
-                $results[$key] = $array[$key];
-            }
-        } else {
-            foreach ((array) $keys as $key) {
-                $results[] = $array[$key];
-            }
-        }
-
-        return $results;
+        return array_slice(static::shuffle($array), 0, $number, $preserveKeys);
     }
 
     /**
@@ -708,15 +695,30 @@ public static function set(&$array, $key, $value)
      */
     public static function shuffle($array, $seed = null)
     {
-        if (is_null($seed)) {
-            shuffle($array);
-        } else {
+        if (! is_null($seed)) {
             mt_srand($seed);
             shuffle($array);
             mt_srand();
+
+            return $array;
         }
 
-        return $array;
+        $keys = array_keys($array);
+
+        for ($i = count($keys) - 1; $i > 0; $i--) {
+            $j = random_int(0, $i);
+            $temp = $keys[$i];
+            $keys[$i] = $keys[$j];
+            $keys[$j] = $temp;
+        }
+
+        $shuffled = [];
+
+        foreach ($keys as $key) {
+            $shuffled[$key] = $array[$key];
+        }
+
+        return $shuffled;
     }
 
     /**

diff --git a/tests/Support/SupportArrTest.php b/tests/Support/SupportArrTest.php
@@ -731,7 +731,26 @@ public function testRandom()
         $this->assertCount(2, array_intersect_assoc(['one' => 'foo', 'two' => 'bar', 'three' => 'baz'], $random));
     }
 
-    public function testRandomOnEmptyArray()
+    public function testRandomIsActuallyRandom()
+    {
+        $values = [];
+
+        for ($i = 0; $i < 100; $i++) {
+            $values[] = Arr::random(['foo', 'bar', 'baz']);
+        }
+
+        $this->assertContains('foo', $values);
+        $this->assertContains('bar', $values);
+        $this->assertContains('baz', $values);
+    }
+
+    public function testRandomNotIncrementingKeys()
+    {
+        $random = Arr::random(['foo' => 'foo', 'bar' => 'bar', 'baz' => 'baz']);
+        $this->assertContains($random, ['foo', 'bar', 'baz']);
+    }
+
+        public function testRandomOnEmptyArray()
     {
         $random = Arr::random([], 0);
         $this->assertIsArray($random);
@@ -817,6 +836,14 @@ public function testShuffleWithSeed()
         );
     }
 
+    public function testShuffle()
+    {
+        $this->assertNotSame(
+            Arr::shuffle(range(0, 10)),
+            Arr::shuffle(range(0, 10))
+        );
+    }
+
     public function testSort()
     {
         $unsorted = [