[RFC] No PHP tags

I forgot to mention 2nd. I usually disable engine for upload directory
by httpd.conf
or do not allow to upload anything under webroot.

Right, you don't put your upload dir under your webroot. Even with php
disabled, you wouldn't want your upload dir in your web root since the
bad guys could upload nasty javascript or other interesting things and
xss/csrf your users through that.

As for LFI, I usually just set my open_basedir to the directories I know
my application will access files from. So even if I make a mistake
somewhere, the bad guys won't be able to trick any of my includes into
including any other files. This seems like a much simpler and more
effective way to combat LFI than introducing a template mode.

-Rasmus

Hi Lester,

I think the RFC is too long, so I'll reorganize it. Old contents remains as
reference
to that people could understand past discussions.

Yasuo Ohgaki wrote:

I like the way PHP works. Embedded script nice for web applications and we
should not drop it, but it's better for security if it is restricted to
only where it is needed.

OK Re-reading the whole RFC ... it is all about 'template_mode' being used
to switch tag handling on and off? PHP 'Open' tag being optional seems to
be secondary?

---

function render_template($template, $template_vars) {
ini_set('template_mode', 'on); // Older PHP ignores
// Do what ever required as template
ini_set('template_mode', 'off'); // Older PHP ignores
}

When switching on template_mode what actually happens? Is it actually in
PHP mode so that you have to add a ?> before loading raw html and then
'close' with a <?php before switching template mode off. How does one
'include' a raw html file with embedded PHP? The include needs to be in PHP
space, but needs to open the file in html mode?

Template mode may be specified only when PHP is started, IMO. (INI_SYSTEM)
Current RFC is INI_ALL, though. If users would like to use
template_mode=on,
they use include()/require() as usual. This is simpler than current RFC.

User may do for any PHP version.

function render_template($template, $template_vars) {
include($template);
}

Anyway, current RFC allows template_mode=off/of at anytime, and it should
not change include()/require() behavior at all. That's the reason why I
propose script()/script_once() for script only include, and it should not
change
behavior depends on template_mode.

Does this answer to you?
I'll reorganize the RFC from now.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi Rasmus,

I think you have enough skill to prevent script inclusion attacks.
However,
all PHP users are not. Take a look at vulnerability reports, you'll find
number of LFI that could be prevented by this RFC.

http://packetstormsecurity.com/search/?q=LFI

These are only reported LFI vulnerabilities and there are countless
vulnerable scripts in the wild.

I don't see any reason to keep doors open for attackers. We are better to
close as many doors as possible, especially for fatal disaster(script
execution).

Thank you for the comment!

Regards,

P.S. I was added script()/script_once() to the original RFC. However, it
may be easier for users to have template()/template_once() that parses
file
as embedded PHP script mode always and keep include()/require() for
script
inclusion. This way, users do not have to rewrite include()/require() to
script(). Anyone has any comments?

I just went through about 20 LFIs at
http://packetstormsecurity.com/search/?q=LFI and in every single case an
appropriately set open_basedir would have prevented the LFI. That seems
pretty definitive to me. The feature to prevent LFI is already there. We
do not need another one. Especially one that introduces a confusing
distinction between scripts and templates which really has nothing to do
with the underlying problem.

So the real problem is two-fold. People are making these mistakes in the
first place, and second, they are not coding defensively and setting
open_basedir. You are still asking people to actively make a defensive
coding change with this RFC, but the people who make these mistakes and
don't code defensively aren't suddenly going to change so they would not
benefit.

I agree.
People who realize importance of security must develop application
defensively. They must validate all inputs strictly when inputs are
supplied. They must use all protection mechanism available.

It's perfectly true that users can write robust web applications with PHP,
but Packet Storm reports show that we are better to have yet another
protection for disasters. People write weak applications/codes. I audit
Rails application also. Rails is not a magic platform that makes web
applications secure. Those people write weak applications with Rails would
write weak applications with PHP, too. I'm sure many of them would write
LFI vulnerable codes. Yet, their applications do not have fatal disaster
like LFI.

PHP is for the web. Web is extremely dangerous as serious professional
attackers exploit web for money. Introducing yet another effective counter
measure for fatal error is valuable for PHP community because there are
vulnerable applications/codes. I cannot accept the fact that PHP could be
much insecure than others when the same developers write applications...

<?php
ini_set('template_mode', 'OFF'); // All require()/include() are protected
against typical LFI
require($some_module); // script only
require($another_module); // script only

// Need template(embedded) mode. Extra caution here.
template($mytemplate . '.php'); // embedded script.

Users may have LFI with template() still, but attack vector is limited to
template(). It's much easier for users to pay extra attentions to limited
template() rather than all script inclusions. (It's easier to audit, too :)

Perhaps, we may be better to force template() to have ".php" extension (or
2nd required param as extension), so that
template($template);
includes "${template}.php" always. It's much secure thanks to your(Rasmus)
null byte attack protection.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi Rowan,

On Sat, Feb 15, 2014 at 2:10 AM, Rowan Collins rowan.collins@gmail.comwrote:

But if somebody is using include() for non-PHP data, they're using
the wrong function anyway, end of story.

Right. This is what I would like to change.
In other languages, such script inclusion ends up syntax error, end of
attack.

I was talking here about my cases 3 and 4, where the author is
deliberately using include for something other than code. There is
already a correct function for that, which is readfile(), or more likely
file_get_contents() and then some processing, so no changes are needed to
introduce it.

3. the include was intended to be non-PHP data, and the attacker
   substitutes PHP code of their choice
4. the include was intended to be non-PHP data, and the attacker accesses
   different non-PHP data already on server

These are abuse of feature. It's better to disable such usage.

There are number of ways to inject piece of PHP code into files. Attacker

can exploit other feature/vulnerability to take over server completely.
Session file is one of the common method.

Are session files relevant to this discussion, or was this just a side
note?

Yes, it is relevant as PHP executes fragment of code in a file always. Many
codes save user supplied data in session. i.e. It can be used for LFI
attack that executes arbitrarily codes .

Compile error will not display any code/contents except a piece displayed

as error message.

That's not quite true. A fatal error can occur after output has been
flushed through to the browser. The include statement will succeed (acting
like readfile()), and the fatal error, if any, will happen afterwards, e.g.
because an expected class definition was not loaded.

When a language executes data files as program, it ends up with syntax
error during compile time almost always. PHP compiles script, then
executes. Program may output something before data file inclusion, but it
must be intended output. Data file inclusion results in syntax error and
content would not be exposed.

Attacker may disguise script a data file. Insufficient data file validation
may permit script execution in other languages, too. I'm not concerned much
about this case. New RFC will address this issue, though.

Take a look at vulnerability reports, you'll find number of LFI that

could be prevented by this RFC.

I looked at some of the examples in the RFC, but I don't understand how
they relate to this proposal. No amount of links to vulnerability reports
can tell me that, but you can.

What I am asking for is an example - real or artificial, I don't mind -
with an explanation of how it behaves now, and how that behaviour would be
improved by the proposed change.

Note that I'm not trying to shoot down your proposal - on the contrary,
I'm trying to make sure its benefits are clearer, so that people will have
a better means of judging it.

I'll abandon this RFC since I came up with better idea. New RFC will
address all issues have been discussed in this and other threads. All
discussions were valuable. Thanks a lot to all!

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net