Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108673
MIME-Version: 1.0
References: <CAFv4g+Ecztuzf_WZXzJ=PNK2R8nSND2mB9eWrR4Z7+5OaVb+hg@mail.gmail.com>
 <CAEYWF=74Atwth0yX5dxR3ZJypVaG1xeYZHz6eV6LoS7gBLkeVg@mail.gmail.com>
 <CAFv4g+HMoE7YMSP=CWBO2WxxKrRaF5Gf6pZcdy_G57UC74Wjrw@mail.gmail.com> <CAEYWF=76-8r6csd_x5NARDmV=BxneeD1y0Cof+4LuXDDEsQP2Q@mail.gmail.com>
In-Reply-To: <CAEYWF=76-8r6csd_x5NARDmV=BxneeD1y0Cof+4LuXDDEsQP2Q@mail.gmail.com>
Date: Wed, 19 Feb 2020 16:49:42 +0000
Message-ID: <CAFv4g+G+TJgpAAVu2ByFTUN-JL3hoUWU=6a+jnKPr0Z1-BznjQ@mail.gmail.com>
To: Bishop Bettini <bishop@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000dcb299059ef093d5"
Subject: Re: [PHP-DEV] $_FILES['name'] check
From: craig@craigfrancis.co.uk (Craig Francis)

--000000000000dcb299059ef093d5
Content-Type: text/plain; charset="UTF-8"

On Wed, 19 Feb 2020 at 16:42, Bishop Bettini <bishop@php.net> wrote:

> On Wed, Feb 19, 2020 at 10:29 AM Craig Francis <craig@craigfrancis.co.uk>
> wrote:
>
>> On Wed, 19 Feb 2020 at 05:23, Bishop Bettini <bishop@php.net> wrote:
>>
>>> On Sun, Feb 16, 2020 at 6:24 PM Craig Francis <craig@craigfrancis.co.uk>
>>> wrote:
>>>
>>>> Just to check, at the moment, if I was an evil hacker, and was to run:
>>>>
>>>> curl -F 'file=@example.jpg;filename=../../../example.php'
>>>> https://example.com/upload/
>>>>
>>>> The $_FILES['file']['name'] would be set to "example.php", where PHP has
>>>> removed the leading "../../../" (good to see).
>>>>
>>>> Does that happen simply because of this IE fix, where it uses
>>>> _basename()
>>>> in the PHP source:
>>>>
>>>>
>>>> https://github.com/php/php-src/blob/0b4778c377a5753a0deb9cfc697d4f62acf93a29/main/rfc1867.c#L1144
>>>
>>>
>>> Mostly, it seems. _basename will either be php_ap_basename[1] or
>>> php_mb_rfc1867_basename[2], and both of those handle the base name
>>> functionality regardless of platform.
>>>
>>> The comment's a little misleading, though. The original
>>> implementation[3] had a magic quotes check when compiled under WIN32, and
>>> that's what the comment's talking about. The comment's not saying that the
>>> basename call itself is for Windows only.
>>>
>>> [1]:
>>> https://github.com/php/php-src/blob/0b4778c377a5753a0deb9cfc697d4f62acf93a29/main/rfc1867.c#L558
>>> [2]:
>>> https://github.com/php/php-src/blob/2e97ae91c8ac404be00050eef414b555aba45a1c/ext/mbstring/mbstring.c#L852
>>> [3]:
>>> https://github.com/php/php-src/blob/7ee1fdb657f2a6da65087552e6dda8cf2f4bd1ef/main/rfc1867.c#L1088
>>>
>>
>>
>>
>> Thanks Bishop,
>>
>> That's interesting, so the comment probably should be updated.
>>
>> I don't think it matters where PHP is compiled, as I'm more focused on
>> what the browser sends to the server.
>>
>> Personally I'd like the comment to mention the security value it
>> provides, as I've seen a few systems that don't
>> pass $_FILES["file"]["name"] though basename(); and if this behaviour was
>> to change (e.g. when "IE's user base drops to nill"), that would introduce
>> a problem.
>>
>>
>> https://stackoverflow.com/questions/18929178/move-uploaded-file-function-is-not-working
>>
>
> I've updated this comment ([1]) to reflect that basename-ing is mandatory
> for RFC 7857 multipart/form-data processing of filename parameters ([2]).
>
> Thank you for helping improve PHP!
>
> [1]:
> https://github.com/php/php-src/commit/fb57ae9084a98ac5f06cd7b2d10205489b537e20
> [2]:https://tools.ietf.org/html/rfc7578
>



Thanks again Bishop.

--000000000000dcb299059ef093d5--