Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87879
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain thomasbley.de from 85.13.137.24 cause and error)
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit
User-Agent: ALL-INKL Webmail 2.11
To: me@kelunik.com
Cc: scott@paragonie.com, internals@lists.php.net, ircmaxell@gmail.com
Message-ID: <20150823153705.F0BDD26052F@dd15934.kasserver.com>
Date: Sun, 23 Aug 2015 17:37:05 +0200 (CEST)
Subject: Re: [PHP-DEV] [RFC] [Discuss] Random Functions Throwing Exceptions in PHP 7.0.0
From: mails@thomasbley.de ("Thomas Bley")

Niklas Keller wrote on 23.08.2015 16:30:

> 
> 
>> why not have false + e_warning for strict_types=0 and fatal error for strict_types=1 ?
>> 
>> 
>> Doing function random_int(): int { ...
> 
> 
> How's this connected to `strict_types`? It's not.
> 
> 
>> If people use this function without reading documentation, they will also use other things without documentation like database queries without binding/escaping, inject html without escaping, etc.
>> Having core functions suddenly throw exceptions causes many problems in the code structure.
> 
> 
> How are these things connected? How does this create any issues in any existing code structure? This RFC affects only two new functions introduced in PHP 7.
> 
> 
>> I think there are a lot of security problems if people ignore return values, e.g. password comparison, user lookup in database, lookups for permissions, etc.
> 
> 
> You compare an edge case, where these two functions currently return false instead of throwing an exception to fail closed, to functions with an expected `true|false` return value.
> 
> 
> This change is especially important, because these functions may be used in a way like this, as already mentioned in the previous discussions:
> 
> 
> for ($i = 0; $i < 10; $i++) {
> 
>     $result .= $values[random_int(0, 10)];
> 
> }
> 
> 
> It's simply far too easy to make mistakes in security relevant code.
> 
> 
> Regards, Niklas
> 
>

> How's this connected to `strict_types`? It's not.

consider this code:

declare(strict_types=0);
ini_set('display_errors', '1');

function get_random_int(): int {
return false;
}
echo get_random_int();

and then use strict_types=1


> How are these things connected? How does this create any issues in any existing code structure? This RFC affects only two new functions introduced in PHP 7.

People will switch their code from mt_rand() to random_int(). So you'll need try-catch in places where you normally not use try-catch.


> for ($i = 0; $i < 10; $i++) {
> 
>     $result .= $values[random_int(0, 10)];
> 
> }

Even correct return values of random_int() might create bad passwords.
So I propose to have a function in core which tests the strength of the password:

$password = '';
for ($i = 0; $i < 10; $i++) {
  $password .= $characters[random_int(0, 30)];
}
if (password_strength($password) < PHP_PASSWORD_STRONG) {
    throw new Exception("password not strong enough");
}

Regards
Thomas