# $_FILES['name'] check - Thread: https://externals.io/message/108624 - Original archive: https://news-web.php.net/php.internals/108624 - Started by Craig Francis on 2020-02-16T23:24:05+00:00 - Messages: 5 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #108624 — Craig Francis — 2020-02-16T23:24:05+00:00 URL: https://externals.io/message/108624#108624 Hi, 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 Craig --- ## #108663 — Bishop Bettini — 2020-02-19T05:22:50+00:00 URL: https://externals.io/message/108624#108663 · In reply to: #108624 On Sun, Feb 16, 2020 at 6:24 PM Craig Francis 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 --- ## #108670 — Craig Francis — 2020-02-19T15:29:00+00:00 URL: https://externals.io/message/108624#108670 · In reply to: #108663 > On Sun, Feb 16, 2020 at 6:24 PM Craig Francis > wrote: > > > […] > > > 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 --- ## #108672 — Bishop Bettini — 2020-02-19T16:41:41+00:00 URL: https://externals.io/message/108624#108672 · In reply to: #108670 On Wed, Feb 19, 2020 at 10:29 AM Craig Francis wrote: > > > > […] > > > > 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 --- ## #108673 — Craig Francis — 2020-02-19T16:49:42+00:00 URL: https://externals.io/message/108624#108673 · In reply to: #108672 > On Wed, Feb 19, 2020 at 10:29 AM Craig Francis > wrote: > > > […] > > 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.