Add FILTER_VALIDATE_INCLUDE validation filter for variable includes

php.internals

Arnold Daniels

7 years ago
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

Andrey Andreev

7 years ago
Hi, On Thu, Sep 20, 2018 at 1:37 PM, Arnold Daniels <arnold.adaniels.nl@gmail.com> wrote:
> 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? >
Those security issues arise from using user inputs as includes at all, not just from lack of sanitization. Sanitization is more often than not imperfect and there's always the potential to bypass it. The only acceptable way of doing something similar to variable includes is the following: if ($input === 'foo') { require 'path/to/foo.php'; } ... which is not in fact a variable include. Adding the filter that you're proposing would imply that doing input variable includes is ok, and I am very strongly against this. Cheers, Andrey.

Rowan Collins

7 years ago
Hi Arnold, Please remember to click "Reply All" / "Reply List" rather than just "Reply", to make sure the list is included in your replies. Right now, most of us are only seeing half the conversation: https://externals.io/message/103196 Cheers,
-- Rowan Collins [IMSoP]

Andrey Andreev

7 years ago
Hi, On Fri, Sep 21, 2018 at 3:03 PM, Rowan Collins <rowan.collins@gmail.com> wrote:
> Hi Arnold, > > Please remember to click "Reply All" / "Reply List" rather than just > "Reply", to make sure the list is included in your replies. Right now, most > of us are only seeing half the conversation: > https://externals.io/message/103196 >
Weird, I see the list as a recipient in his replies ... Maybe something's blocking him? Cheers, Andrey.

Markus Fischer

7 years ago
On 21.09.18 14:07, Andrey Andreev wrote:
> On Fri, Sep 21, 2018 at 3:03 PM, Rowan Collins <rowan.collins@gmail.com> wrote: >> Hi Arnold, >> >> Please remember to click "Reply All" / "Reply List" rather than just >> "Reply", to make sure the list is included in your replies. Right now, most >> of us are only seeing half the conversation: >> https://externals.io/message/103196 >> > > Weird, I see the list as a recipient in his replies ... Maybe > something's blocking him?
I didn't see this reply either, only yours. Maybe you received them because you were directly addressed? 🤷‍♀️ cheers, - Markus

Andrey Andreev

7 years ago
Hi, On Sat, Sep 22, 2018 at 12:35 PM, Markus Fischer <markus@fischer.name> wrote:
> > On 21.09.18 14:07, Andrey Andreev wrote: >> >> On Fri, Sep 21, 2018 at 3:03 PM, Rowan Collins <rowan.collins@gmail.com> >> wrote: >>> >>> Hi Arnold, >>> >>> Please remember to click "Reply All" / "Reply List" rather than just >>> "Reply", to make sure the list is included in your replies. Right now, >>> most >>> of us are only seeing half the conversation: >>> https://externals.io/message/103196 >>> >> >> Weird, I see the list as a recipient in his replies ... Maybe >> something's blocking him? > > > I didn't see this reply either, only yours. Maybe you received them because > you were directly addressed? 🤷‍♀️ >
Again - he's got internals@lists.php.net in CC for both of his replies so far. There's been a few occasions where gmail has marked newcommers' messages to the list as spam for me, but I don't remember if I had to click Not Spam on this one ... can you guys check your Spam dirs? Cheers, Andrey.

Rowan Collins

7 years ago
On 22 September 2018 10:54:53 BST, Andrey Andreev <narf@devilix.net> wrote:
>Hi, >There's been a few occasions where gmail has marked newcommers' >messages to the list as spam for me, but I don't remember if I had to >click Not Spam on this one ... can you guys check your Spam dirs?
That wouldn't explain it not showing up on public archives, though; that's why I linked to externals.io, and they're all missing from marc.info too: https://marc.info/?t=153744017200004&r=1&w=2 It could be that the From address is different on the replies versus the first message, so the list is seeing it as unregistered? I know I've made that mistake before. Either that or the list is deciding they're spam for some reason. Regards,
-- Rowan Collins [IMSoP]

Arnold Daniels

7 years ago
Sorry about that. I'll be more careful when replying. - Arnold On Fri, Sep 21, 2018 at 2:07 PM Andrey Andreev <narf@devilix.net> wrote: