PHP Application Insecurity - PHP or Devs Fault?

somersault asks: "There have recently been a lot of people making jokes at the expense of PHP, but how many common security flaws in PHP are the fault of the language, and how many the fault of the developer? A recent Security Focus article (via the Register) has a brief discussion which suggests that PHP is no less secure than any other scripting language, and that it is the users of the language themselves who need to be educated. The other side of the story is that the developers of PHP should work on tightening up the language to make it more 'idiot proof' by default. Should the team developing PHP take a more active role in controlling the use of their language? What will it take to ensure that users of the language learn to use it securely, short of defacing every vulnerable website out there?"

Comments

The problem is ....

[lambent]

The problem is that so many neophyte progrrammesr jump into PHP to create something visible and useful. Which they succeed in doing, more often that not, I guess. But without a proper background in security and proper practice, there's a ton of vulnerabilities that get created, accidentally, over and over again by every new PHP programmer.

The same can be said about any other language. Take for instance, C. Very easy to create working code that's vulnerable as hell. Is this the original author's fault? Of course not. I'm sorry that whoever chose to write a webapp in PHP is ignorant of basic security principals, but it's not up to the coders of PHP to protect us from ourselves.

Re:The problem is ....

[Coryoth]

If you're not good enough to sanitize, error check, bounds check, mem check, fault check, and whatever the hell else could go wrong, you have no business coding.

I think the point is that we're all human and we all make occasional mistakes even with the best of intentions. There's plenty of code out there written by very experienced C programmers that still has buffer overflows and other glitches. That means that having a language that has the facility to make such errors easier to catch and correct early is a good thing. That means that having a language that pushes you toward secure practice by not having sloppy easy ways to do things is a good. Yes, we could all, in theory, write perfectly secure error free code in C or PHP or whatever, but in practice we don't - no one does. Languages that encourage best practice by default and provide the tools to catch errors earlier (with, say, design by contract) are a good thing if security is important. We're all human, and can use all the help we can get.

This is easy to test empirically

[Schraegstrichpunkt]

Take 100 programmers selected randomly, and instruct them all to write a given application, but have 20 of them write the code in PHP, 20 write the code in Python, 20 write the code in Java, and 20 write the code in C++, and 20 write the code in Perl. Then analyze the resulting code.

Re:Tool safety

[Beryllium Sphere(tm)]

The aviation industry began making real safety improvements when they stopped regarding "pilot error" as the end of the story and began to fix ergonomics so that pilots weren't led into error.

SQL escaping considered evil...

[numbsafari]

I kind of agree with where you are going, but I would add the following point:

SQL Escaping is evil.

Why?

Because no user input should ever be executed. EVEN if it is escaped. The problem is that the escaping can be invalid and buggy and thus, insecure.

People should use parametric SQL statements. No excuses. In this manner, no escaping is ever necessary.

A separate issue is what to do about displaying user input. Here, things are more problematic, especially in the world of HTML. What would be nice is if we all got together and redesigned "the web" so that user input could be handled in a manner similar to parameters in SQL.

Obviously, there's a difference between data in tables and data in a formatted page. But I'm sure something could be done.

Who's fault? Zend's

[kestasjk]

I've audited quite a lot of PHP, written an article on PHP security from the hackers perspective, and done quite a lot of PHP development, and I've never come across an security problem that you could blame the developers for!

It's always the developer assuming something about PHP or the PHP environment but getting it wrong; you can argue that the developer should know, but there are so many gotchas in PHP, you have to be an expert to be aware of them all. (I've listed some in a previous post on /. , and won't repeat myself here).
This isn't right for any language, but a language which web applications run on?! The most hostile environment to develop for is not the place for a language that makes it so easy to trip up!

The fault, for the vast majority of PHP security problems, is completely Zend's. Zend needs to give security priority over backwards compatibility, and get rid of all of their problems that developers repeatedly trip up on.

Re:Who's fault? Zend's

[kestasjk]

magic_quotes and register_globals, which are options that can be turned off: register_globals is off by default, and has been that way for a long time. Anyone who turns it back on deserves what they get. It's a dead issue. magic_quotes is headed for the same fate in PHP 6. They seemed like good ideas at the time the web as young; they turned out not to be.

Yes, but if you develop on a server which has magic_quotes on, and you deploy on a server that has it off, your code won't behave as expected. You have to be aware of magic_quotes before writing anything in PHP, if you want to be safe.
Same goes for register_globals; and it's hardly a non-issue as it's enabled just about everywhere in the name of backwards compatibility. In the article I wrote the site that got exploited had a vulnerability exposed by register_globals.

"Only critical errors are reported...unless you specifically turn up the error_reporting level" Configurable logging and reporting is a feature, not a bug.

You bet it's a bug when only critical errors are reported by default. Errors in code aren't shown, and users don't realize that there's a problem in their code until it's being exploited.

"fopen_urls: By default you can include scripts hosted on other websites!" I'll agree, that should probably be off by default. But a developer has to be naive or dim to either use an URL include, or include a variable in the include directive (and thus introduce the possibility of a URL inclusion) without being damn sure what they're doing.

I don't think you can blame the developer for this. If they develop with magic_quotes on, or register_globals off, or error reporting >E_WARNING, they may not realize the variable in the include string is writeable, and they probably wouldn't realize you can include remote documents anyway.

"Input checking is difficult...Do you want htmlentities() or htmlspecialchars()?" Depends on what you want to do, now, doesn't it? Developers have to know what conditions they need their data to adhere to, and PHP gives them a variety of tools to make it fit those conditions. Feature, not a bug.

What about add_slashes() not escaping everything that mysql_escape() does? Or mysql_escape() not escaping everything that mysql_real_escape() does? What about 5 == "5 OR 1=1"? What about the ability to input arrays (and errors which should be shown when dealing with unexpected arrays aren't printed because of the default error reporting level)? These are bad ideas which make sanitizing input difficult.

It's easier to trip up badly in C (by commiting some memory buffer error) or Perl (by writing line noise code that you can't understand a week later) than PHP. But it's no longer fashionable to bash those languages.

I wouldn't use C for the web either, and Perl can be very clear. I agree that PHP gets a worse rep than it deserves; I like PHP, and understand that if bash or C was the language of the web they'd be just as bad, but they're not and PHP is.
PHP would be so much better if they fixed the security holes; one of the reasons it gets such a bad rep is because it lets newcomers make mistakes so easily, I'd like PHP to be recognized as the excellent language it is but these security problems aren't helping.

Apparently what you see as "problems", others see as features.

Some see pointers and no bounds checking as useful features, but that doesn't mean they're a good idea for security.

Re:Who's fault? Zend's

[TheLink]

Definitely Zend's fault!

From your post: "magic_quotes: This adds slashes to all input so that you don't have to sanitize it before it gets inserted into SQL."

BUT that is so totally the WRONG thing to do and a MISFEATURE, and the fact that the PHP developers made it a big feature of PHP shows why they and PHP suck. Think I'm being harsh? Read on.

This is what any sane programmer should do:

Each input source for YOUR application should be _individually filtered and escaped so that _YOUR_ application can handle the inputs correctly.

Each output destination for your application should be _individually_ filtered and escaped[1] so that the RECEIVING programs/entities can handle your app outputs correctly.

Example:
Say some data is http posted to a PHP web app, and the PHP app then sends the resulting data to a MySQL database, an Oracle database, syslog, and in some cases also emails some of that data to an email address, or redisplays the data in an HTML form on a web browser (required field left out).

magic_quotes would add slashes to the data when it enteres the web app, and that CORRUPTS the data. The resulting munged data _might_ still work for MySQL, but as is be incorrect for Oracle and SMTP (<lf>.<lf> needs to become <lf>..<lf>), data to syslog should have ctrl chars removed or escaped _appropriately_ and to be safe kept < 1024 bytes in length, and data to an HTML form shouldn't have the added slashes, but instead be appropriately quoted for HTML.

My proposal would have the web app filtering/escaping the data so the webapp can handle it, and then escape/filter stuff appropriately for MySQL, Oracle, SMTP, syslog and HTML. It seems like more work, but it is the correct way. It is less work in the long run especially if you make/use the appropriate libraries.

Once you understand the above, you should see why magic_quotes is so TERRIBLE, and why I have a low opinion of PHP and Zend.

And magic_quotes is not the only PHP misfeature that makes PHP PHP. You have named a few already.

Basically PHP makes doing the wrong thing easy, but the right things hard[2].

[1] by escaping/filtering I also include use of "SQL prepared/parameterized statements".

[2] After all these years it's still not clear what DB abstraction layer/library to use for PHP - there's the PDO vs PEAR DB thing, and PHP users are still resorting to crap like addslashes and magic_quotes. If each PHP coder writes their own DB library, anyone else taking over has to learn it. PHP should have learnt from the other languages mistakes.

For perl you use DBI, for Java you use JDBC.

All this crappiness has to be blamed on the developers who made PHP.

Re:I have to agree..

[quadra23]

PHP does not give you a pretty library with prepared statements, parameter binding, and such. There's a nice DB and MDB2 package available on PEAR, but PHP doesn't ship with those. It ships with the compile option --with-mysql.

Perl ships with a fair amount of stuff. It ships with a package named DBI. You can do things like $rv = $sth->execute(@bind_values);. The documentation on it starts off with a convenient set of good examples which go like

$sth = $dbh->prepare("SELECT foo, bar FROM table WHERE baz=?"); $sth->execute( $baz );

PHP 5.2 ships with PDO (PHP Data Objects) extension which can run with your example code provided you load the extension in your php.ini (yes, I know its a setting that is not done by default, but that argument doesn't hold water with PHP 5 anymore). PDO also supports the prepared statements and parameter bindings of which you speak, and along similar lines, you can also do transactions. You should be clear about which version of PHP your referring to as PHP 4.4 is no longer considered the main release and also has not been updated since August while PHP 5 was last updated in November.

Though I can still agree that not all the choices made in the development were the best. AFAIK, every language has human developers and humans are not perfect, but we do do the best we can and have to continually aim to improve ourselves and the work we create.

Re:mysql_escape_string, mysql_real_escape_string,

[AdamPiotrZochowski]

FIRST : stop forcing prepared binded statements for all :

I dont mind prepared statements for when they are usefull, but they dont always work properly. And actually there are many cases where using them you actually lose power. Lets start with a simple example of the LIKE clause :

SELECT * FROM titles WHERE notes LIKE ?

For the unfamiliar, like clause allows me to do partial searches over strings (char/varchar in the sql world). The LIKE clause search string syntax is something of a simplified regular expression. This means that characters that usually have one meaning gain another one. For example the percentage sign becomes a wildcard (think dos/bash filename matching with '*', or regexp with '.*'). For example, all string starting with 'word' we would just search for 'word%'. Great, but how does prepare/binded statement know if the given percentage is to be escaped or not. It doesnt. So you end up doing own user parsing. You are back to square one. You need to still parse user input, so whats the point of binded/prepared statement? Another example is using power provided through fulltext index. Generally, string searching is slow. In SQL world we do an index, a cache to speed up looking. Strings have indexes, but that only speeds up searching for string that start with something (like in above example LIKE 'word%') but what if we want to search for something purely inside the string ?? then we could do LIKE '%word%' but thats slow, on the other hand, we could speed this up by various smart caching and indexing of the contents of the string. This smart indexing we call 'full text'. For example to see if a column contains some word or phrase we could just do

SELECT * FROM myData WHERE CONTAINS (column, ?)

all ok, right? NOPE, because it also could be :

SELECT * FROM myData WHERE CONTAINS (column, 'FORMSOF (INFLECTIONAL, ?)')

To explain slightly, the second examples tries to find words that are not exact, but very close. So for word 'good' another word 'best' could be used as an alternative (with a lower relevancy ranking). Great power?? Yes, but the first time the sql expects the query in the form CONTAINS ( notes , ' "word" ') notice single and double quotes while later its CONTAINS(notes, 'FORMSOF (INFLECTIONAL, word)') notice, no quotes allowed...

and dont even get me started with the

SELECT * FROM myData WHERE column IN ( ? )

The IN clause is a speed over a series of OR statements. I could write WHERE column = 1 OR column = 2 OR column =3 or I could just do it with WHERE column IN ( 1,2,3) . And now the question for the binding gurus. How do I do it with prepared statements ?? Do I create a loop and both generate the SQL and fill a flat array with the right amount of paramenters WHERE column IN ( ? , ? , ? , ? ) , or do I just send arrays within arrays.

SECOND : parameter binding through naming :

cant wait for when parameter binding can be done in a templated fashion, so that no longer order of the columns matters, currently the way you fill prepared statement with data matters by order of the data. It all should be done with associative arrays.

$sth = $__db->prepare ( "select * from myData where cond1 = ? and cond2 = ? " ) ;
$res =& $__db->execute ( $sth , array ( $userInput1 , $userInput2 ) ) ;

it should be done more like

$sth = $__db->prepare ( "select * from myData where cond1 = ?userInput1 and cond2 = ?userInput1 " ) ;
$res =& $__db->execute ( $sth , array ( "userInput1" => $userInput1 , "userInput2" => $userInput2 ) ) ;

There is no special need to input more -- if you want, use the first method just pass non associative array, and library should know to handle param binding in old way -- but for any larger querry, with dozens of parameters, this will be a big boon in readab

Speaking as a PHP Framework Developer

[Foofoobar]

I used to work with the Zend team and they seem determined o pander to the least common denominator of hobbiests and not allow the language to grow up. Things like nested classes and strongly types variab;es which should have been implemented in the latest version are strongly fought against. They things as well as other would help enforce good coding standards. But I have been told by the Zend developers themselves that they like to leave it up to the developer to code badly and to me that makes the language just as much to blame. I think the industry has established by now what are good programming habits and methodologies and what aren't.

Re:Tool safety

[VGPowerlord]

Have you ever used PHP? If not, take a look at the following features:

1. addslashes() function - Most references say to use this to quote data before putting it into a database. Not only is it The Wrong Way To Do It (twwtdi) according to the SQL standard, but different vendors' databases need different values escaped.
2. Magic Quotes ini settings. magic_quotes_gpc is the most important one. When enabled, it runs addslashes() on all GET, POST, and cookie input. It is on by default in php.ini-dist, off by default in php.ini-recommended. Which brings up my next point...
3. The programming environment is not consistent. An INI file controls the programming environment. Turning on things like Safe mode and open_basedir can cause previously working code to suddenly fail. Of course, php.ini-dist has display errors turned on by default, so anyone visiting that page will see the location of your file, the line that has the error, and which error it is... Which brings up the next point:
4. Security is secondary to convenience. See Using remote files, which is enabled in both ini files by default. See also Magic Quotes as described earlier. Reading up on the deprecated Register Globals is informative, as it was on by default up until PHP 4.2.0. I've also mentioned display_errors, which is on in php.ini-dist.
   
   Here's a disturbing fact: php.ini-dist is the more ocmmonly used of the two inis, at least for shared hosting. I'll let you consider the implications of that while I summarize things.

To summarize: The PHP team has made a number of questionable decisions over the years that makes it much easier to write a security hole than it should be.

Re:Tool safety

[Cecil]

There are two sides to that coin, you know. Aircraft designers tend to be extremely conservative, yes. But there's a reason for that.

Why do general aviation planes usually have extremely simplistic, 4-stroke big block engines with carbureators? Because they generally work. They do fail, but their failure modes are very well known, very obvious, and usually easy to fix. Those are important qualities, and I'm willing to bet that you underrate them. If you replaced the aircraft's engine with a modern fuel injected digitally controlled model, what happens if an injector clogs, or the computer goes insane? No one knows, and they're not eager to find out. If your carb ices, you can fix that. In-air, no less. It may be a less reliable design overall, but the failure modes are usually pretty tame. And that's worth a lot to an aircraft designer.

Taking one of your examples regarding the stall indicator/yoke... do you really want to take a piece of equipment which in correct operation is almost *never* going to get used, and hook it up to your primary controls? That's just *asking* for trouble. If the stall indicator ever gets jammed open (it is just a little metal flap after all, it's unlikely but possible), your "safety" measure may well crash the plane on its own.

It's not as easy as it looks. These people are not idiots, they simply have a lot of variables to consider and weigh. And they have a pretty solid track record behind them, too.