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
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_basename1 or
php_mb_rfc1867_basename2, and both of those handle the base name
functionality regardless of platform.
The comment's a little misleading, though. The original implementation3
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.
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_basename1 or
php_mb_rfc1867_basename2, and both of those handle the base name
functionality regardless of platform.The comment's a little misleading, though. The original implementation3
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.
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
On Wed, Feb 19, 2020 at 10:29 AM Craig Francis craig@craigfrancis.co.uk
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_basename1 or
php_mb_rfc1867_basename2, and both of those handle the base name
functionality regardless of platform.The comment's a little misleading, though. The original implementation3
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.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"]
thoughbasename(); 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!
On Wed, Feb 19, 2020 at 10:29 AM Craig Francis craig@craigfrancis.co.uk
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_basename1 or
php_mb_rfc1867_basename2, and both of those handle the base name
functionality regardless of platform.The comment's a little misleading, though. The original
implementation3 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.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"] thoughbasename(); 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!
Thanks again Bishop.