On Wed, 19 Feb 2020 at 16:42, Bishop Bettini <bis...@php.net> wrote: > On Wed, Feb 19, 2020 at 10:29 AM Craig Francis <cr...@craigfrancis.co.uk> > wrote: > >> On Wed, 19 Feb 2020 at 05:23, Bishop Bettini <bis...@php.net> wrote: >> >>> On Sun, Feb 16, 2020 at 6:24 PM Craig Francis <cr...@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.
