Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve error reporting in random extension #9071

Merged
merged 10 commits into from
Jul 25, 2022
Merged

Improve error reporting in random extension #9071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8daa5fe
Use `php_random_bytes_throw()` in Secure engine's generate()
TimWolla Jul 20, 2022
b7bdf1a
Use `php_random_int_throw()` in Secure engine's range()
TimWolla Jul 20, 2022
f93daeb
Throw exception when a user engine returns an empty string
TimWolla Jul 20, 2022
fdfa2f3
Throw exception when the range selector fails to get acceptable numbe…
TimWolla Jul 20, 2022
12b07ce
Improve user_unsafe test
TimWolla Jul 20, 2022
3e18441
Fix user_unsafe test
TimWolla Jul 20, 2022
8adb67a
Remove duplication in user_unsafe.phpt
TimWolla Jul 20, 2022
3090379
Catch `Throwable` in user_unsafe.phpt
TimWolla Jul 21, 2022
39b996a
Throw an error if an engine returns an empty string
TimWolla Jul 24, 2022
734c79c
Throw an Error if range fails to find an acceptable number in 50 atte…
TimWolla Jul 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions ext/random/engine_secure.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ static uint64_t generate(php_random_status *status)
{
zend_ulong r = 0;

if (php_random_bytes_silent(&r, sizeof(zend_ulong)) == FAILURE) {
if (php_random_bytes_throw(&r, sizeof(zend_ulong)) == FAILURE) {
status->last_unsafe = true;
}

Expand All @@ -38,9 +38,9 @@ static uint64_t generate(php_random_status *status)

static zend_long range(php_random_status *status, zend_long min, zend_long max)
{
zend_long result;
zend_long result = 0;

if (php_random_int_silent(min, max, &result) == FAILURE) {
if (php_random_int_throw(min, max, &result) == FAILURE) {
status->last_unsafe = true;
}

Expand Down
1 change: 1 addition & 0 deletions ext/random/engine_user.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ static uint64_t generate(php_random_status *status)
result += ((uint64_t) (unsigned char) Z_STRVAL(retval)[i]) << (8 * i);
}
} else {
zend_throw_error(NULL, "A random engine must return a non-empty string");
status->last_unsafe = true;
return 0;
}
Expand Down
8 changes: 6 additions & 2 deletions ext/random/random.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ static zend_object_handlers random_engine_xoshiro256starstar_object_handlers;
static zend_object_handlers random_engine_secure_object_handlers;
static zend_object_handlers random_randomizer_object_handlers;

#define RANDOM_RANGE_ATTEMPTS (50)

static inline uint32_t rand_range32(const php_random_algo *algo, php_random_status *status, uint32_t umax)
{
uint32_t result, limit, r;
Expand Down Expand Up @@ -118,7 +120,8 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat
/* Discard numbers over the limit to avoid modulo bias */
while (UNEXPECTED(result > limit)) {
/* If the requirements cannot be met in a cycles, return fail */
if (++count > 50) {
if (++count > RANDOM_RANGE_ATTEMPTS) {
zend_throw_error(NULL, "Failed to generate an acceptable random number in %d attempts", RANDOM_RANGE_ATTEMPTS);
status->last_unsafe = true;
return 0;
}
Expand Down Expand Up @@ -174,7 +177,8 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat
/* Discard numbers over the limit to avoid modulo bias */
while (UNEXPECTED(result > limit)) {
/* If the requirements cannot be met in a cycles, return fail */
if (++count > 50) {
if (++count > RANDOM_RANGE_ATTEMPTS) {
zend_throw_error(NULL, "Failed to generate an acceptable random number in %d attempts", RANDOM_RANGE_ATTEMPTS);
status->last_unsafe = true;
return 0;
}
Expand Down
200 changes: 137 additions & 63 deletions ext/random/tests/03_randomizer/user_unsafe.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,81 +3,155 @@ Random: Randomizer: User: Engine unsafe
--FILE--
<?php

// Empty generator
$randomizer = (new \Random\Randomizer(
new class () implements \Random\Engine {
public function generate(): string
{
return '';
}
}
));
use Random\Randomizer;

try {
var_dump($randomizer->getInt(\PHP_INT_MIN, \PHP_INT_MAX));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
final class EmptyStringEngine implements \Random\Engine {
public function generate(): string
{
return '';
}
}

try {
var_dump(bin2hex($randomizer->getBytes(1)));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
final class HeavilyBiasedEngine implements \Random\Engine {
public function generate(): string
{
return "\xff\xff\xff\xff\xff\xff\xff\xff";
}
}

try {
var_dump($randomizer->shuffleArray(\range(1, 10)));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
}
echo "=====================", PHP_EOL;

try {
var_dump($randomizer->shuffleBytes('foobar'));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
}
foreach ([
EmptyStringEngine::class,
HeavilyBiasedEngine::class,
] as $engine) {
echo $engine, PHP_EOL, "=====================", PHP_EOL, PHP_EOL;

// Infinite loop
$randomizer = (new \Random\Randomizer(
new class () implements \Random\Engine {
public function generate(): string
{
return "\xff\xff\xff\xff\xff\xff\xff\xff";
}
try {
var_dump((new Randomizer(new $engine()))->getInt(0, 123));
} catch (Throwable $e) {
echo $e, PHP_EOL;
}

echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;

try {
var_dump(bin2hex((new Randomizer(new $engine()))->getBytes(1)));
} catch (Throwable $e) {
echo $e, PHP_EOL;
}

echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;

try {
var_dump((new Randomizer(new $engine()))->shuffleArray(\range(1, 10)));
} catch (Throwable $e) {
echo $e, PHP_EOL;
}

echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;

try {
var_dump((new Randomizer(new $engine()))->shuffleBytes('foobar'));
} catch (Throwable $e) {
echo $e, PHP_EOL;
}
));

try {
var_dump($randomizer->getInt(\PHP_INT_MIN, \PHP_INT_MAX));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
echo PHP_EOL, "=====================", PHP_EOL;
}

try {
var_dump(bin2hex($randomizer->getBytes(1)));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
}
?>
--EXPECTF--
=====================
EmptyStringEngine
=====================

try {
var_dump($randomizer->shuffleArray(\range(1, 10)));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
}
Error: A random engine must return a non-empty string in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->getInt(0, 123)
#1 {main}

try {
var_dump($randomizer->shuffleBytes('foobar'));
} catch (\RuntimeException $e) {
echo $e->getMessage() . PHP_EOL;
}
Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->getInt(0, 123)
#1 {main}

-------

Error: A random engine must return a non-empty string in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->getBytes(1)
#1 {main}

Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->getBytes(1)
#1 {main}

-------

Error: A random engine must return a non-empty string in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
#1 {main}

Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
#1 {main}

-------

Error: A random engine must return a non-empty string in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
#1 {main}

Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
#1 {main}

=====================
HeavilyBiasedEngine
=====================

Error: Failed to generate an acceptable random number in 50 attempts in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->getInt(0, 123)
#1 {main}

Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->getInt(0, 123)
#1 {main}

-------

?>
--EXPECTF--
Random number generation failed
Random number generation failed
Random number generation failed
Random number generation failed
int(%d)
string(2) "ff"
Random number generation failed
Random number generation failed

-------

Error: Failed to generate an acceptable random number in 50 attempts in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
#1 {main}

Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
#1 {main}

-------

Error: Failed to generate an acceptable random number in 50 attempts in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
#1 {main}

Next RuntimeException: Random number generation failed in %s:%d
Stack trace:
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
#1 {main}

=====================