Hacker News Reader

Comment by jorkro | Hacker News Reader

It's important to note that the author seems to be flagging injections based on the use of a PHP variable in the query string ( https://github.com/laurent22/so-sql-injections/blob/master/s... ).

Although a strong indicator, this does not necessarily imply an SQLi. If the variable is sanitized before the query (for instance, the variable is cast to an int), there is no SQLi to exploit.

So it's very possible that these graphs are riddled with false positives.

View on HN

It is more than a strong indicator.

But I think the absolute % is missing the point. What I find interesting is the evolution. It doesn't go down.

PHP has always been easy to pick up, so it's always going to have amateurs. These questions riddled with vulnerabilities are usually asked by people that don't understand what they are doing nor want to learn; it's not really representative of the sector of developers that make a living with PHP.

Having a lot of amateurs is one thing. What I am bit concerned about is that the alarming pace of major data leaks, many of which start with an amateurish mistake like this, should increase the awareness to this kind of flaws.

The evolution of this chart seems to suggest that this is not the case, at least not for the wider, mainstream community (I am not referring to a minority of MIT/Stanford-educated elite computer scientists).

I have had four zero-days affecting 5.5 million devices sitting on a public code repository for two months now, for a project maintained by dozens of corporations employing high-end PHP programmers, who do write PHP for a living.

The fixes are code reviewed, but not merged, because the developers don't seem to understand PHP-into-C null string terminator vulnerabilities, or type juggling, or strict comparison, or... I could go on.

PHP is unsafe at any speed, because it almost invites arbitrary code execution through a number of vectors. It isn't inherently bad if used correctly, as most Facebook developers will tell you, but the language structure involves quite a number of insecure practices.

After all, most programmers don't expect:

<?php 0 == "string"; ?> to be true.

> After all, most programmers don't expect: <?php 0 == "string"; ?> to be true.

nobody would expect that

Experienced Perl programmers would, and PHP took inspiration (altough in crippled way) from Perl.

Anyone who ever programs in a weakly-typed language should be aware that comparing two values with different types will involve type coercion, that there are obviously strange edge cases when that happens, and that comparing with a strict comparison operator such as === is good defensive programming in those contexts.

Wait, is GP saying that IS true in php?!?!

Yeah, go check in a sandbox.

It gets worse: https://3v4l.org/Slvpp

Note that PHP also has a JavaScript-style triple-equals comparison, which does not attempt type conversion and does not exhibit this bizarre behaviour.

My favorite is that two obviously different hexadecimal values in strings (e.g. checking password hashes) can be "equal" with the weaker == comparison.

This occurs if PHP thinks both strings could be numbers in scientific notation. "0e123" == "00e45"

> It isn't inherently bad if used correctly

Is BAD. Imagine if a car was made with the same "design".

A tool is BAD if the user must patch to overcome the inherent behaviour it show.

The parent poster already referenced "unsafe at any speed" so I think they are already aware of the car analogy :)

Can I see a link? Just curious about the vulnerabilites. What's a PHP-into-C null string terminator vulnerability?

Google null-byte injection.

As for the framework, that's RDK-B. http://rdkcentral.com http://code.rdkcentral.com https://github.com/rdkcmf

The deeper you look, the worse it gets. Those php issues are very trivial, first glance type stuff. Some need a bit of a twist to make exploitable, but another will strip the encryption right off the hidden network.

I have others I wish to disclose, but I can't seem to get them to respond to my requests for a PoC. Quite frankly, I'm shocked that I can't seem to get anyone to realize how serious the impact of an RCE vulnerability in a framework fielded that widely truly is.

If you find any of more serious things I'm talking about on your own, wait for the vendors to fix them. Please don't brick the world.

>PHP-into-C null string terminator vulnerabilities

They should still be fixed, but I believe these bugs are no longer an issue in PHP after 5.3.

PHP's problem isn't ease of entry so much as the materials provided to new users, which are awful. The official tutorial desperately needs rewriting and if you google 'php tutorial' you'll find pages of outdated insecure advice.

> it's not really representative of the sector of developers that make a living with PHP.

Anecdote: I was integrating a paid Magento extension (which charges about $1000 for a license) into an existing shop, and some poor code in their templates made me look deeper into their main code. Six hours later, I had:

* 4 different ways to read any file on the server that PHP can read,

* 5 different ways to upload any file to a certain directory on the server, including one that lets you overwrite the webserver's security configuration for that directory and then execute uploaded PHP code,

* a way to delete certain things the administrator has created in the backend,

* a way to overwrite other customers' uploaded files,

* a way to edit other customers' information, which lets you then XSS/XSRF those customers when they view that information.

I have reviewed other Magento extensions in the past, and I found many poor ones, but nothing as atrocious as this.

If you (or anyone) ever sees anything like this please email marketplace@magento.com or me at ben@magento.com.

Probably you encountered this in the days of Magento Connect. Our new app store (Marketplace) actually hosts the code and has static code tests for many things, including OWASP items.

Thank you for the contact details, I'll send you the details of the security issues later this week.

This extension was purchased through Magento Connect just a few months ago. I'm glad to hear the situation is improving now.

Ruby is extremely easy to pick up, but since the community tries to steer people towards frameworks like Rails where making an injection vulnerability is non-trivial, requires actual understanding and deliberate effort, the injection problems are relatively rare.

For those that use a framework for PHP they're also insulated from these problems, they have better examples to work from, but the PHP community is full of people that think they're too good for frameworks, or that frameworks are an obstacle to understanding.

Those people are the cause of so much damage.

Yes there are definitely some false positives (as well as false negatives), though the lack of prepared statements and use of concatenated strings like this are usually a good indicator that there's in fact an SQL injection vulnerability. If you manually check some of these questions, you'll often find that the parameters come from $_POST or other user input, so that's why I didn't try to make it any smarter since just checking for "some sql... some $variable" is a good enough indicator.

I would at least tweak the regexes to not match dynamic table names (select FROM $table, UPDATE $table, etc). That's a very common pattern that you can't solve with placeholders.

Wouldn't it make sense to strip out a lot of unnecessary code to get to the point faster and allow others to reproduce the issue at hand easier? Just because the question is omitting validation and sanitation doesn't mean it has flaws. Isn't that the nature of SE questions? If your question is about a piece of logic deep down in your stack, you're likely not going to post 3 layers of code above it.

Hmm, could be, but I think if someone cares enough to write a minimal working sample, they'll also care enough not to have SQL injection vulnerabilities in their code (if only because people might lecture them about this instead of answering the question :). More often than not these issues are found in code dump like this[0] where nothing has been cleaned up.

[0] https://stackoverflow.com/questions/40958763

Reply on news.ycombinator.com