Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108670
MIME-Version: 1.0
References: <CAFv4g+Ecztuzf_WZXzJ=PNK2R8nSND2mB9eWrR4Z7+5OaVb+hg@mail.gmail.com>
 <CAEYWF=74Atwth0yX5dxR3ZJypVaG1xeYZHz6eV6LoS7gBLkeVg@mail.gmail.com>
In-Reply-To: <CAEYWF=74Atwth0yX5dxR3ZJypVaG1xeYZHz6eV6LoS7gBLkeVg@mail.gmail.com>
Date: Wed, 19 Feb 2020 15:29:00 +0000
Message-ID: <CAFv4g+HMoE7YMSP=CWBO2WxxKrRaF5Gf6pZcdy_G57UC74Wjrw@mail.gmail.com>
To: Bishop Bettini <bishop@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000361aae059eef73c3"
Subject: Re: [PHP-DEV] $_FILES['name'] check
From: craig@craigfrancis.co.uk (Craig Francis)

--000000000000361aae059eef73c3
Content-Type: text/plain; charset="UTF-8"

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

Craig

--000000000000361aae059eef73c3--