5
\$\begingroup\$

I want a cryptographically secure version of array_rand(). Is this it?

/**
 * retrieve a random key from an array, using a cryptographically secure rng.
 * - it does the same as array_rand(), except that this one use a cryptographically secure rng.
 * - relatively speaking, it should be significantly slower and more memory hungry than array_rand(), because it is creating a copy of all the keys of the array..
 *
 * @param array $arr
 * @throws ValueError if array is empty
 * @return mixed
 */
function array_rand_cryptographically_secure(array $array) {
    if (count ( $array ) < 1) {
        throw new ValueError ( 'Argument #1 ($array) cannot be empty' );
    }
    if (PHP_MAJOR_VERSION >= 8 && array_is_list ( $array )) {
        // optimization, this avoids creating a copy of all the keys
        return random_int ( 0, count ( $array ) - 1 );
    }
    $keys = array_keys ( $array );
    return $keys [random_int ( 0, count ( $keys ) - 1 )];
}

Significant edit: I figured I can use array_is_list() to avoid copying the keys when given a list.

\$\endgroup\$

3 Answers 3

4
\$\begingroup\$
  1. array_is_list() is available from PHP8.1. You can check the PHP version major&minor version with version_compare() (version_compare(PHP_VERSION, '8.1', '>=')) or function_exists() -- the latter is more concise and explicit than the former.
  2. The $max key position is shared by both branches of the conditional logic, so you can count() just once. In other words, regardless of if you call array_keys(), the array's size will not change.
  3. With $max already declared, $keys becomes a single-use variable and is therefore omittable.
  4. I prefer to type hint whenever possible. Since array keys can only either be integers or strings, the return value can be typed. With PHP8, union types allow the explicit dual-typing of the return value as a string or an integer. Of course, if you are trying to make a function that works under PHP8, then disregard this point.

Code: (Demo)

function array_rand_cryptographically_secure(array $array): int|string {
    $max = count($array) - 1;
    if ($max < 0) {
        throw new ValueError('Argument #1 ($array) cannot be empty');
    }
    if (function_exists('array_is_list') && array_is_list($array)) {
        // optimization, this avoids creating a copy of all the keys
        return random_int(0, $max);
    }
    return array_keys($array)[random_int(0, $max)];
}

$tests = [
    [5, 6, 7],
    ['a' => 1, 'b' => 2, 'c' => 3],
    ['zero', 4 => 'four', 9 => 'nine']
];
foreach ($tests as $test) {
    echo array_rand_cryptographically_secure($test) . "\n";
}

Alternatively, you can slice and preserve the original keys to avoid doubling the memory cost. (Demo)

function array_rand_cryptographically_secure(array $array): int|string {
    $max = count($array) - 1;
    if ($max < 0) {
        throw new ValueError('Argument #1 ($array) cannot be empty');
    }
    return key(array_slice($array, random_int(0, $max), 1, true));
}

And finally, the most compact and perhaps hardest to read because of all of the nested function calls: (Demo)

function array_rand_cryptographically_secure(array $array): int|string {
    if (!$array) {
        throw new ValueError ('Argument #1 ($array) cannot be empty');
    }
    return key(array_slice($array, random_int(0, count($array) - 1), 1, true));
}
\$\endgroup\$
2
\$\begingroup\$
  1. This is not API compatible with array_rand, as it doesn't have the ability to return more than one key (but array_rand does). This may or may not matter for your application, but this is not a generic "cryptographically secure version of array_rand()". Specifically, it is attempting to be a cryptographically secure version of array_rand for just the single key case.

  2. The array_is_list function is not appropriate for use in cryptographic security, as it does not guarantee a constant time answer. So if you want to use that, you should write your own function (e.g. constant_time_array_is_list) or note that your function is not safe from timing attacks (i.e. better define what you mean when you say it is cryptographically secure). Just as hash_equals is the timing attack safe version of strcmp. But strcmp is fine if both strings are publicly known.

\$\endgroup\$
0
\$\begingroup\$

Here is a better version. It's almost similar to the array_rand, it accepts both $array and $num to pick one or more random keys out of an array. Also, all the validations are included in this function.

Code (Demo)

function array_rand_secure($array, $num = 1)
{
    if (!is_array($array)) {
        trigger_error(
            __FUNCTION__ . "(): expects parameter 1 to be array, " . gettype($array) . " given",
            E_USER_WARNING
        );
        return null;
    }
    $arrayLength = count($array);
    if ($arrayLength === 0) {
        trigger_error(__FUNCTION__ . "(): Array is empty", E_USER_WARNING);
        return null;
    }
    if (
        filter_var($num, FILTER_VALIDATE_INT) === false
        || $num <= 0
        || ($arrayLength - $num) < 0
    ) {
        trigger_error(
            __FUNCTION__ . "(): Second argument has to be between 1 and the number of elements in the array",
            E_USER_WARNING);
        return null;
    }

    $randomIndexes = [];
    while(count($randomIndexes) < $num) {
        $randomInt = random_int(0, $arrayLength - 1);
        if (!in_array($randomInt, $randomIndexes)) $randomIndexes[] = $randomInt;
    };

    $arrayKeys = array_keys($array);
    $randomKeys = array_map(function ($key) use ($arrayKeys) {
        return $arrayKeys[$key];
    }, $randomIndexes);

    return count($randomKeys) === 1 ? $randomKeys[0] : $randomKeys;
}


$tests1 = [
    [1, 2, 3, 4, 5, 6, 7, 8, 9],
    [5, 6, 7],
    ['a' => 1, 'b' => 2, 'c' => 3],
    ['zero', 4 => 'four', 9 => 'nine'],
    ["PEAN"=>0],
    [],
    "ABC",
    123,
];
foreach ($tests1 as $test) {
    var_dump(array_rand_secure($test));
}

$tests2 = [
    ["array" => [1, 2, 3, 4, 5, 6, 7, 8, 9], "num" => 9],
    ["array" => [1, 2, 3, 4, 5, 6, 7, 8, 9], "num" => 4],
    ["array" => ['a' => 1, 'b' => 2, 'c' => 3], "num" => 2],
    ["array" => ['zero', 4 => 'four', 9 => 'nine'], "num" => 2],
    ["array" => ["PEAN" => 0], "num" => 2],
    ["array" => [], "num" => 2],
    ["array" => "ABC", "num" => 2],
    ["array" => 123, "num" => 2],
    ["array" => [5, 6, 7], "num" => 0],
];
foreach ($tests2 as $test_data) {
    var_dump(array_rand_secure($test_data["array"], $test_data["num"]));
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Welcome. Please explain why is it a better version ? \$\endgroup\$ Commented Jan 25, 2024 at 11:23
  • 1
    \$\begingroup\$ Supplying source code is fine, but a Review on stack exchange code review should definitely include some critique of the code that OP submitted. Maybe you'd like to highlight some OP code that neglected a validation, and describe the Bad Things that could result from that. \$\endgroup\$
    – J_H
    Commented Jan 25, 2024 at 18:44
  • \$\begingroup\$ Better in the sense it can accept both $array and $num to pick one or more random keys out of an array. Also, all the validations are included in this function. \$\endgroup\$
    – Kunal Ray
    Commented Feb 8, 2024 at 5:05

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.