Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:103465
To: Stanislav Malyshev <smalyshev@gmail.com>,
 PHP Internals <internals@lists.php.net>,
 "release-managers@php.net" <release-managers@php.net>
References: <47dd2988-6fb2-0685-efa1-a58cb55e9ecb@gmail.com>
Message-ID: <563360e1-aa2f-6a2f-7299-caf9b85f2fff@gmx.de>
Date: Tue, 20 Nov 2018 23:41:43 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.3.1
MIME-Version: 1.0
In-Reply-To: <47dd2988-6fb2-0685-efa1-a58cb55e9ecb@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: de-DE
Content-Transfer-Encoding: 7bit
Subject: Re: patch for imap bug 77153
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 20.11.2018 at 20:45, Stanislav Malyshev wrote:

> Strictly speaking, such bug is a problem in the library, not PHP
> wrapper, since all parsing and mailbox string handling is done inside
> the library and it completely opaque to PHP. However, c-client library
> has been essentially unsupported for many years (why we're using an
> ancient unsupported library is a separate issue which we'd probably want
> to address but let's not get distracted) so no fix is probably coming
> from that direction. And since imap extension is used by a bunch of
> tools and most are not aware underlying library has this vulnerability,
> I think disabling this function is a right thing to do. More details in
> the bug and in the UPGRADING note.

I fully agree with the fix (thanks!), and also that it is a security
issue.  However, I don't think it's really a problem in c-client;
actually the PHP wrapper should not have allowed to pass the mailbox
name verbatim, which would only be reasonable in my opinion, if we were
supporting arbitrary drivers (which we don't).  And of course, userland
clients should not pass unvalidated input as mailbox name, but as you
said, quite likely at least some developers are not aware that
potentially arbitrary shell commands could be executed this way, and our
docs don't explicitly mention this issue.

> For RMs, please incorporate it into the next release. Maybe not that
> urgent for PHP 7.3.0RC6 since it's not a production release anyway.

PHP-7.3.0 has already been branched, and PHP-7.2.13RC1 and PHP-7.3.0RC6
have already been tagged without the patch.  We probably should re-tag.

-- 
Christoph M. Becker