Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72893
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.219.43 as permitted sender)
MIME-Version: 1.0
Date: Sun, 2 Mar 2014 01:12:58 +0100
Message-ID: <CAF+90c-M+0-Rbqxx_MSK-3ffifS2tM68rztsSmOa0ovWek20Zw@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7bd6c52c869d5804f394880c
Subject: Stricter error handling in mcrypt extension
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7bd6c52c869d5804f394880c
Content-Type: text/plain; charset=ISO-8859-1

Hi internals!

I would like to add a number of additional error checks in
php_mcrypt_do_crypt - which affects the mcrypt_encrypt, mcrypt_decrypt and
mcrypt_{BLOCK_CHAINING_MODE} userland functions.

The proposed changes are:
 * Throw a warning and return bool(false) if the IV size is invalid. The
old behavior was to throw a warning and use a NUL-byte IV.
 * Throw a warning and return bool(false) if no IV was specified, but the
block chaining mode requires an IV. The old behavior was to throw a warning
and use a NUL-byte IV.
 * Throw a warning and return bool(false) if the key size is invalid. The
old behavior was to **silently** pad the string to the next valid key size
with NUL bytes or, if the key is too long, to throw a warning and truncate
it to the maximum valid key size.

An implementation of these changes can be found in the PR
https://github.com/php/php-src/pull/610.

The reason why I'd like to make the mcrypt input handling stricter is to
ensure that incorrectly implemented encryption will fail and fail
violently. With the current lax input checking it is very easy to
completely compromise confidentiality through simple mistakes - like using
a password as the encryption key.

Thanks,
Nikita

PS: I'm running this change by the list because stricter error handling is
technically a BC break. Of course this change is targeting PHP 5.6 only.

--047d7bd6c52c869d5804f394880c--