Add FILTER_VALIDATE_INCLUDE validation filter for variable includes

There are many security issues that arise from not sanitizing a variable
before using it in an include (eg include $script;).

The filter extension is intended to prevent this kind of security issues. A
validation filter would make it easier and could be the defacto standard
when using variable includes.

When a static code analyzer is used, it can check if the filter has been
used and the variable is safe to be used in include.

The options could be "base_path, allowed_streams".

The base_path option defines the path where the file should be in. Dots
like .. are resolved. Home paths, like ~/foo and ~arnold/ are not
allowed (or resolved). Symlinks are not considered.

The allowed_streams option would set which streams are allowed. By
default none. I feel this is a better option than relying on
'allow_url_include' or RFC: Precise URL include control (
https://wiki.php.net/rfc/allow_url_include).

include filter_var($script, FILTER_VALIDATE_INCLUDE, ["base_path" =>

"path/to/project/", "allowed_streams" => ["phar", "zip"]]);

What do you think?

Also, does this require an RFC or should I just create a PR?

• Arnold

Hi again,

Variable includes have proper purposes, like for a (PSR-4) autoloader. This
can't be simply replaced with an 'if' statement. Other reasons are module
inclusion and generated code.

Of course, there are a few valid applications even for the most
discouraged practices - in that you are correct. But you should know I
meant user input variable inclusion in particular.
Either way, I don't see how a user input filter validator would help
PSR-4, generated code and/or whatever you meant by module inclusion.

Just to be clear, in everything I'm saying, I assume you want to solve
the following problem:

include $_GET['page'].".php"; // <-- vulnerability

(simplified, of course)

Variable inclusion is already done very often. I don't think this filter
will persuade people to do it that would otherwise not. This is a common
security issue. So if variable inclusion isn't disabled in full, having a
common way to prevent such issues seem like a good idea to me.

Well, I've got two things to say about this:

1. The developers who intruduce such vulnerabilities are the ones who
   don't do validation in the first place, so the way I see it, the few
   poor souls who might benefit from your proposal wouldn't care for it
   anyway.
2. Reiterating from my previous reply, this would only serve as an
   excuse for some to say that user input variable inclusion is an OK
   thing to do, because "see, PHP has a tool specifically for it, so it
   must be good".

Sanitization is more often than not imperfect and there's always the
potential to bypass it.

This would be a validation filter and not a sanization filter. Can you give
an example on how you could bypass it?

Sorry, you started the discussion by mentioning sanitization and I
just went with it without noticing the details.

Still, even as a validator this can be bypassed depending on the
application architecture - sure, you specify a base path, but who is
to say I'm not trying to RCE via something within that base path?

Cheers,
Andrey.

Hi,

Hi again,

Variable includes have proper purposes, like for a (PSR-4) autoloader.
This
can't be simply replaced with an 'if' statement. Other reasons are
module
inclusion and generated code.

Of course, there are a few valid applications even for the most
discouraged practices - in that you are correct. But you should know I
meant user input variable inclusion in particular.
Either way, I don't see how a user input filter validator would help
PSR-4, generated code and/or whatever you meant by module inclusion.

With module inclusion, I mean the way Symfony works with Bundles and
Wordpress works with plugins.

I'm familiar with neither, sorry.

To be honest, few applications seem to have static include statements at
all. It's all dynamic.

Again, talking about user inputs. Yes, most includes are dynamic, but
still controlled by the developer and not with user-supplied strings.
In fact, since you mentioned PSR-4 which is used for pretty much
everything nowadays, you should know that almost nobody writes
"include" anymore and it's all handled by the autoloader in a
bullet-proof manner.

Just to be clear, in everything I'm saying, I assume you want to solve
the following problem:

include $_GET['page'].".php"; // <-- vulnerability

(simplified, of course)

With all the abstraction you see in modern applications, it's difficult to
tell if a variable could have been influenced via the HTTP request. It's
better always to validate the variable before using it in the include.

... and I noted that was a simplified example.

But while it is true that many variables are influenced by the HTTP
request, I fail to recall a valid case where they're not already
validated by a router mapping variables to pre-defined classes,
already handled by an autoloader.

This is getting kind of hard to follow, we may not even be talking
about the same thing at this point. Can you provide an example of a
case where your proposed filter would solve a problem?

Variable inclusion is already done very often. I don't think this filter
will persuade people to do it that would otherwise not. This is a common
security issue. So if variable inclusion isn't disabled in full, having
a
common way to prevent such issues seem like a good idea to me.

Well, I've got two things to say about this:

1. The developers who intruduce such vulnerabilities are the ones who
   don't do validation in the first place, so the way I see it, the few
   poor souls who might benefit from your proposal wouldn't care for it
   anyway.

If there is a proper way to do it, they'll probably be more inclined to do
it. It's the same as for SQL injection. There is a right and a wrong way to
inject variables.

That's just wishful thinking; I've never seen a developer who doesn't
do validation all of a sudden go "oh, there's this validator, I should
use it!". They don't do it because they don't care about it, not
because tools don't exist ... we already have tools for everything
that the average developer needs. And not that it matters, but SQLi
prevention is quite different.

2. Reiterating from my previous reply, this would only serve as an
   excuse for some to say that user input variable inclusion is an OK
   thing to do, because "see, PHP has a tool specifically for it, so it
   must be good".

There are probably not many scripts where you see direct user input being
used for include. It's always an indirect hack.

If you link the 'filter' extension to request data, perhaps the function
should not be in this extension.

I don't understand what you're saying here ... I thought you were
trying to solve a security problem, now all of a sudden that's not
related to request data? I'll wait for that example I asked for.

Sanitization is more often than not imperfect and there's always the
potential to bypass it.

This would be a validation filter and not a sanization filter. Can you
give
an example on how you could bypass it?

Sorry, you started the discussion by mentioning sanitization and I
just went with it without noticing the details.

Still, even as a validator this can be bypassed depending on the
application architecture - sure, you specify a base path, but who is
to say I'm not trying to RCE via something within that base path?

You should not allow uploading files with base_path. Current RFCs aren't
solving this issue and I don't see how to solved this, short of disalowing
variable for includes completely. But that's unacceptable since most
applications rely on it.

"Should not" doesn't mean people don't do it, and I wasn't talking
about uploaded files anyway. Either way, that's a moot point ... my
position is that there's no value in the proposed feature, regardless
of whether it can be bypassed or not.

Cheers,
Andrey.