# [RFC][DISCUSSION] Introduce scrpt_path - Thread: https://externals.io/message/82040 - Original archive: https://news-web.php.net/php.internals/82040 - Started by Yasuo Ohgaki on 2015-02-06T12:02:53+00:00 - Messages: 10 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #82040 — Yasuo Ohgaki — 2015-02-06T12:02:53+00:00 URL: https://externals.io/message/82040#82040 Hi all, This RFC was renamed from "script() and script_once()". Original proposal had defect. It wasn't perfect. This RFC proposes "script_path" INI directive to eliminate file/script inclusion at all via require(). https://wiki.php.net/rfc/script_path It's work in progress, but I would like to start discuss. Thank you! Regards, P.S. Pierre, discussion with you was very helpful. Thank you. -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #82041 — Leigh — 2015-02-06T12:53:47+00:00 URL: https://externals.io/message/82040#82041 · In reply to: #82040 > Hi all, > > This RFC was renamed from "script() and script_once()". > Original proposal had defect. It wasn't perfect. > > This RFC proposes "script_path" INI directive to eliminate > file/script inclusion at all via require(). > > https://wiki.php.net/rfc/script_path Couple of fixes and questions > Introduce script_path and download_path > Introduce script_path INI that specify directories that execution is allowed and upload_path Change download_path to upload_path. > PHP script detection by file content is **NOT** impossible > Therefore, PHP script detection by file content is impossible. I assume that you want to say impossible in both places? > include()/require() is source of file/script inclusion for a long time > Only require()/require_once() is affected. Is this on purpose? Is there a reason that include is not affected? I think this is a better solution than script{,_once}. I definitely prefer it over the previous RFC --- ## #82056 — Matthew Leverton — 2015-02-06T16:45:06+00:00 URL: https://externals.io/message/82040#82056 · In reply to: #82040 > https://wiki.php.net/rfc/script_path > > It's work in progress, but I would like to start discuss. > I don't really like it; but I don't really like most INI-based solutions. I agree that this is a problem with poorly written code, but at the same time I don't know that a good solution is to compensate in this regard. Bad programmers will always be bad programmers. Now we're giving them a crutch that almost encourages unchecked includes by relying on an INI configuration. What happens if you are using multiple libraries and they are each overriding their (and your) settings? It just feels messy. The same thing could be accomplished config-less with a callback system that is triggered on script includes (if any callback returns false, an error is thrown), but I think I'd dislike that for similar reasons. -- Matthew Leverton --- ## #82059 — Yasuo Ohgaki — 2015-02-06T18:46:20+00:00 URL: https://externals.io/message/82040#82059 · In reply to: #82041 Hi Leigh, > > > […] > > Couple of fixes and questions > > > […] > allowed and upload_path > > Change download_path to upload_path. > Yes, I'll fix it. > > > […] > > I assume that you want to say impossible in both places? > Yes > > > […] > > Is this on purpose? Is there a reason that include is not affected? > include()/require() supports URI include. This is the reason why. > > I think this is a better solution than script{,_once}. I definitely > prefer it over the previous RFC I thought script()/script_once() is enough, but it's not. There are modules uses custom script loaders, including phar. Those loader may do whatever they want, therefore detecting/deciding file type (i.e. PHP script) by file content is wrong. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #82061 — Yasuo Ohgaki — 2015-02-06T19:02:20+00:00 URL: https://externals.io/message/82040#82061 · In reply to: #82056 Hi Matthew, > > > […] > > I don't really like it; but I don't really like most INI-based solutions. > One possible alternative is use of parser state and use only upload_path. This way, we can remove one INI. > > I agree that this is a problem with poorly written code, but at the > same time I don't know that a good solution is to compensate in this > regard. Bad programmers will always be bad programmers. Now we're > giving them a crutch that almost encourages unchecked includes by > relying on an INI configuration. What happens if you are using > multiple libraries and they are each overriding their (and your) > settings? It just feels messy. > Basically, it's administrative solution. Application should set these setting or administrator should. Library shouldn't touch the setting, otherwise they hit their own foot. > > The same thing could be accomplished config-less with a callback > system that is triggered on script includes (if any callback returns > false, an error is thrown), but I think I'd dislike that for similar > reasons. If upload path is located under docroot, we must use per dir INI and set engine=off This requires administrative task (or Apache should enable .htaccess) Unlike upload_path, this couldn't prevent PHP from loading like require('../../upload/evilfile'); Since my goal is to make PHP as secure as other languages, this case may be ignored. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #82062 — Yasuo Ohgaki — 2015-02-06T19:11:56+00:00 URL: https://externals.io/message/82040#82062 · In reply to: #82059 Hi Leigh, > > > […] > > > I thought script()/script_once() is enough, but it's not. > There are modules uses custom script loaders, including phar. Those loader > may do whatever they want, therefore detecting/deciding file type (i.e. > PHP script) > by file content is wrong. > If parser state is used, script() solution would work and may remove script_path. Then it's possible try to read files as PHP script by require() excluding upload_path/open_basedir/OS restriction. I think this is acceptable. Please note that OS solution does not help to prevent PHP from reading uploaded script. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #82065 — Yasuo Ohgaki — 2015-02-06T19:46:09+00:00 URL: https://externals.io/message/82040#82065 · In reply to: #82040 Hi all, > This RFC was renamed from "script() and script_once()". > Original proposal had defect. It wasn't perfect. > > This RFC proposes "script_path" INI directive to eliminate > file/script inclusion at all via require(). > > https://wiki.php.net/rfc/script_path > > It's work in progress, but I would like to start discuss. > > I would like to know your preference. Multiple choices are OK ( +1 / -1 ) Comments are appreciated. 1. script_path INI. (Defines script path. Almost perfect solution with upload_path INI) [1] 2. upload_path INI. (Exception path in script_path. Protection against require('../../upload/evil');) [1] 3. require_embed INI (Enable/disable require()/require_once() embed(script only) mode. Temp INI) [2] 4. script()/script_once() [3] (No INI switch. Read only scripts. The same as require()/require_once(), require_embed=On ) 5. Leave as it is now (No protection against file inclusion & execution attacks.) [1] script_path defines script directory, upload_path defines exceptions under script_path. [2] require_embed is not described in current RFC. It's INI for enable/disable script only mode. require_embed should be REMOVED few years later. [3] script/script_once is not described in current RFC. It read/execute script only file. Thank you! Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #82067 — Matthew Leverton — 2015-02-06T20:29:59+00:00 URL: https://externals.io/message/82040#82067 · In reply to: #82061 > > Basically, it's administrative solution. Application should set these > setting > or administrator should. > > Library shouldn't touch the setting, otherwise they hit their own foot. > If this was a PHP_INI_PERDIR setting, then I wouldn't really mind as much. But as PHP_INI_USER, I don't like it at all. -- Matthew Leverton --- ## #82127 — Yasuo Ohgaki — 2015-02-08T09:03:06+00:00 URL: https://externals.io/message/82040#82127 · In reply to: #82067 Hi Matthew, > > > […] > If this was a PHP_INI_PERDIR setting, then I wouldn't really mind as much. > > But as PHP_INI_USER, I don't like it at all. How about allow to set the ini only once during execution? To all, please give some feedback. There is option 5(Leave as it is), too! I would like to know your preference. Multiple choices are OK ( +1 / -1 ) Comments are appreciated. 1. script_path INI. (Defines script path. Almost perfect solution with upload_path INI) [1] 2. upload_path INI. (Exception path in script_path. Protection against require('../../upload/evil');) [1] 3. require_embed INI (Enable/disable require()/require_once() embed(script only) mode. Temp INI) [2] 4. script()/script_once() [3] (No INI switch. Read only scripts. The same as require()/require_once(), require_embed=On ) 5. Leave as it is now (No protection against file inclusion & execution attacks.) [1] script_path defines script directory, upload_path defines exceptions under script_path. [2] require_embed is not described in current RFC. It's INI for enable/disable script only mode. require_embed should be REMOVED few years later. [3] script/script_once is not described in current RFC. It read/execute script only file. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #82166 — Rowan Collins — 2015-02-08T20:02:07+00:00 URL: https://externals.io/message/82040#82166 · In reply to: #82127 > > […] > How about allow to set the ini only once during execution? Like open_baedir [1], it should probably be possible to *tighten* the settings, but never *loosen* them. So, you could remove items from script_path, or alter them to be more specific; or you could add them to upload_path, or alter them to be more general. The relationship to open_basedir should perhaps be more explicit - for instance, your example of require('/etc/passwd') is better handled by open_basedir, which also prevents readfile('/etc/passwd'). A file path should ideally be evaluated against 3 tests: A) should this file be accessible, and readable? (controllable through open_basedir; should be a pre-requisite for B and C) B) should this file be writable? (currently left to OS level protection) C) should this file be executable as PHP code? (should apply through include()/require(), and also if passed as a direct argument to the PHP SAPI; the latter part is currently left to the web server configuration, while the first part cannot be controlled at all) Even with all of these, there are still vulnerabilities, just ones which require multiple steps, e.g.: - for any file in (A), read its contents into memory, and trick the application into executing it from a string - find an intersection between (B) and (C), such as a compiled template cache, and inject a string from user input, or a file in (A), which will then be executed [1]: http://php.net/manual/en/ini.core.php#ini.open-basedir -- Rowan Collins [IMSoP]