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
