Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.
I am trying to prepare my Director for a request of mine (that all web development go through our IS department at the very least, if we cannot handle all development) and I have noticed this in some of the code. The PHP set up is handling this pretty well, but are there ways to break this somehow?
The error settings on the PHP install are pretty good, in that I have tried to throw a lot of stuff at it and it just rolls right over it. Basically if the page cannot get included it just displays a page with content where the page would be.
How can I convince my boss that this is not a good way to go? I am thinking that an approach of 'the customer experience may not be to their satisfaction with blank page loads, no input validation and no communication'. I am just thinking that you never ever want to take user input and throw it into an include. But I can't seem to find a way to break this to my boss's satisfaction. Oh well, I guess I am not that good of a hacker...
Everah wrote:How can I convince my boss that this is not a good way to go? I am thinking that an approach of 'the customer experience may not be to their satisfaction with blank page loads, no input validation and no communication'. I am just thinking that you never ever want to take user input and throw it into an include. But I can't seem to find a way to break this to my boss's satisfaction. Oh well, I guess I am not that good of a hacker...
The fact that it creates internal errors alone is enough in my book. Whether they are displayed or not is secondary. Showing a "blank page" is a VERY bad user experience. There's blindly including a page based solely on user input too, which you've already noted. Granted, the potentially REALLY bad stuff is less with this variant on the theme, but it's still a bad practice no matter where you go.
Is it just me being tired, or it's true that with this code you can include (and therefore execute) any php file on the system, provided the serving process has read access to it? Think $_GET["id"] = "../../../path/to/anyfile". This is really bad, even if there is "no way" for the attacker to put his own file on the server.
A bit offtopic about putting files on the server: it's possible for an attacker to make a fake upload. By "fake" I mean that there is no actual form on the web page that would have input=file field. The upload is just a HTTP POST multipart/form-data to any php script. The file ends inside php-upload-dir. It has a randomly generated name and no extension, so it's not a real threat in this (or maybe any) case, but there it is -- a file on the server.
Everah wrote:How can I convince my boss that this is not a good way to go? I am thinking that an approach of 'the customer experience may not be to their satisfaction with blank page loads, no input validation and no communication'. I am just thinking that you never ever want to take user input and throw it into an include. But I can't seem to find a way to break this to my boss's satisfaction. Oh well, I guess I am not that good of a hacker...
The fact that it creates internal errors alone is enough in my book. Whether they are displayed or not is secondary. Showing a "blank page" is a VERY bad user experience. There's blindly including a page based solely on user input too, which you've already noted. Granted, the potentially REALLY bad stuff is less with this variant on the theme, but it's still a bad practice no matter where you go.
Cool. I think we're on the same page here. Thanks feyd.
It's bad practice (and probably fairly common) but limited in terms of what goes wrong.
Is it just me being tired, or it's true that with this code you can include (and therefore execute) any php file on the system, provided the serving process has read access to it?
The prefix prior to the GET variable defeats that - self limiting. Remove the prefix path however...
this code tries to include "folder_name/topic_/../../../dir/file.php", right? Which means that "../dir/file.php" gets included, since "directory/.."-pair cancel each other out. Put in enough "../"s, and you're at the top of the tree. Now all you have to do is climb down.
No, "topic_" doesn't have to exist. I just tested it and it works -- you can include any file on the system, provided that:
- the file ends with ".php",
- you have read access to it.
Thanks guys. On a side note, does anyone know what the default php.ini setting for log_errors is? I think that for the project I am working it would be helpful to have alook at the PHP generated errors. I have never really looked into the PHP error logs so I am not sure if they are on by default and, if they are on, where the log resides.