SQL injections vulnerabilities in Stack Overflow PHP questions(laurent22.github.io)
laurent22.github.io
SQL injections vulnerabilities in Stack Overflow PHP questions
https://laurent22.github.io/so-injections/
114 comments
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.
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.
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.
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
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?!?!
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"
This occurs if PHP thinks both strings could be numbers in scientific notation. "0e123" == "00e45"
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.
Note that PHP also has a JavaScript-style triple-equals comparison, which does not attempt type conversion and does not exhibit this bizarre behaviour.
Yeah, go check in a sandbox.
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.
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.
They should still be fixed, but I believe these bugs are no longer an issue in PHP after 5.3.
> 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.
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.
> 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.
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 [email protected] or me at [email protected].
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.
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.
This extension was purchased through Magento Connect just a few months ago. I'm glad to hear the situation is improving now.
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.
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).
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).
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.
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.
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
[0] https://stackoverflow.com/questions/40958763
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.
Not really, these are just non parametrierend SQL Statements. It doesn't mean that the variables in the SQL Statements have not been sanitized before hand. [1]
It may not be best practice but doesn't automatically mean SQL injection vulnerable.
And this is not limited to PHP
[1] https://github.com/laurent22/so-sql-injections/issues/1
It may not be best practice but doesn't automatically mean SQL injection vulnerable.
And this is not limited to PHP
[1] https://github.com/laurent22/so-sql-injections/issues/1
Yeah, that was my immediate thought. Something like this:
$delete = "DELETE FROM `cart` WHERE id='$id'";
Which is currently among the top three, could be prefaced by the built in mysqli escape function or at minimum cast to (int).
This is not a great approach, the query should be prepared, but it's not inherently an injection risk based on that snippet alone.
And the underlying functions come from the C library for MySQL. Almost any language's SQL libraries will allow you to send an arbitrary string to the database. This is not a PHP problem by any measure.
$delete = "DELETE FROM `cart` WHERE id='$id'";
Which is currently among the top three, could be prefaced by the built in mysqli escape function or at minimum cast to (int).
This is not a great approach, the query should be prepared, but it's not inherently an injection risk based on that snippet alone.
And the underlying functions come from the C library for MySQL. Almost any language's SQL libraries will allow you to send an arbitrary string to the database. This is not a PHP problem by any measure.
Perhaps it's the whole mindset. Why even use that when you have perfectly fine built in mechanisms for SQL parameters for over a decade? I wouldn't even start writing a query like that.
You can do stuff like that too in a .Net / SQL stack for example (or in Django) but somehow everybody defaults to Entity framework or similar solutions that take care of it.
I'm way too lazy to deal with lines and lines of sanitizing, raw queries and mysql_blahblahblah anyway so I write wrappers that give me the results from a query and a bunch of parameters in one line. Secure. Simple.
You can do stuff like that too in a .Net / SQL stack for example (or in Django) but somehow everybody defaults to Entity framework or similar solutions that take care of it.
I'm way too lazy to deal with lines and lines of sanitizing, raw queries and mysql_blahblahblah anyway so I write wrappers that give me the results from a query and a bunch of parameters in one line. Secure. Simple.
If you google "PHP MySQL", you can find all sorts of horrible tutorials like [1] teaching you to write SQL injections. So unlike users of other languages, PHP programmers are taught the wrong way first and then are expected to figure it out somehow.
[1] http://www.freewebmasterhelp.com/tutorials/phpmysql/7
[1] http://www.freewebmasterhelp.com/tutorials/phpmysql/7
That website is from 2001 though
The bigger problem is that Google ranks that website higher than the modern stuff. You can make much better tutorials and it won't help as long as that legacy cruft is sitting at the top of every newbie's search results.
True. I notice that I filter Google results on last year only just to filter out outdated stuff. Just today I wondered why everything on the web (read: Google) was getting so old.
No operation with side effects should ever be run without validation- you could cast to an int, sure, but unless you validate the user is allowed to delete that particular int, you're still giving them free reign over your entire cart table.
The biggest trouble with these questions is that people are learning many aspects of web development without understanding the separate concerns they should be aware of when heading to production code. It's difficult to (a) avoid over-answering and explaining everything while also (b) avoid giving insecure advice
The biggest trouble with these questions is that people are learning many aspects of web development without understanding the separate concerns they should be aware of when heading to production code. It's difficult to (a) avoid over-answering and explaining everything while also (b) avoid giving insecure advice
> you could cast to an int, sure, but unless you validate the user is allowed to delete that particular int, you're still giving them free reign over your entire cart table.
That's a completely different issue and not an SQL injection error though.
That's a completely different issue and not an SQL injection error though.
What should have been invalid input has been turned into valid input, but the valid input doesn't reflect the user's intent. As a result, the behavior is undefined. Best case scenario, nothing happens. Worst case, random rows are deleted.
Personally, I don't think introducing undefined behavior is ever good advice, even if it is technically correct advice.
All of this really just gets at my original point, though: there are so many things for new developers to learn that it is extremely difficult to provide good answers on a forum like SO. That doesn't diminish the value, but we have to be aware of, accept, and make clear this fact.
Personally, I don't think introducing undefined behavior is ever good advice, even if it is technically correct advice.
All of this really just gets at my original point, though: there are so many things for new developers to learn that it is extremely difficult to provide good answers on a forum like SO. That doesn't diminish the value, but we have to be aware of, accept, and make clear this fact.
> Which is currently among the top three, could be prefaced by the built in mysqli escape function or at minimum cast to (int).
OTOH it is entirely unnecessary to quote integers (and the escaping function would not quote it before injecting it), so outlook not so good.
OTOH it is entirely unnecessary to quote integers (and the escaping function would not quote it before injecting it), so outlook not so good.
Yes, it's unnecessary, but it doesn't cause a problem for MySQL.
I've seen lots of people do that though I don't know their rationale. Maybe they just don't know SQL
I've seen lots of people do that though I don't know their rationale. Maybe they just don't know SQL
It's finding some things where placeholders aren't a solution....
29/11/2016 08:23:13: $update = 'UPDATE '.$table.' SET ';
You can't use a placeholder for a table name. Using a dynamic table name is pretty standard practice. The reason for that is to allow a table prefix, allowing multiple of the same app within a single database. And, typically, the table name originates from a config file, not web POSTS/GETS or other "taintable" sources.
29/11/2016 08:23:13: $update = 'UPDATE '.$table.' SET ';
You can't use a placeholder for a table name. Using a dynamic table name is pretty standard practice. The reason for that is to allow a table prefix, allowing multiple of the same app within a single database. And, typically, the table name originates from a config file, not web POSTS/GETS or other "taintable" sources.
Also, one of the examples is:
03/12/2016 19:33:11: $pdo->prepare("UPDATE users SET avatar=? WHERE id=22 ")->execute([$u]) ;
Found the regex's being used: https://github.com/laurent22/so-sql-injections/blob/master/s...
It's pretty basic. For UPDATE, it's matching anything with UPDATE followed by SET, followed by a dollar sign+alphanumeric
And, it's specifically going after dynamic table names. Hmm.
It's pretty basic. For UPDATE, it's matching anything with UPDATE followed by SET, followed by a dollar sign+alphanumeric
And, it's specifically going after dynamic table names. Hmm.
'/UPDATE\s+.*?\$[a-zA-Z0-9].*?\sSET.*?/i', // UPDATE $table SET ...
> You can also escape the scans ...
... or by splitting the SQL string over multiple lines. Or building it up by concatenating strings. Let's stop this now, we've demonstrated that the code is very brittle :)
... or by splitting the SQL string over multiple lines. Or building it up by concatenating strings. Let's stop this now, we've demonstrated that the code is very brittle :)
Another comment mentions that it would skip $_GET['whatever'] and $_POST['whatever'], which is probably in the most egregiously vulnerable code. I think the underscore thing is worth fixing.
> You can also escape the scans by using variable names that start with an underscore
So people who use
So people who use
"WHERE id=" . $_GET['id']
I just did the same :) It flags, for example, the following as an SQL injection:
query("DELETE FROM foo WHERE id=1");$id=2;
I suspect you can fool it by splitting into different lines, which is actually common with longish queries.
$query="SELECT whatever from foo WHERE".
"id=$bar ".
"AND " .
"field=$value";Yes there are false positives. It would be too time consuming to extract and parse the PHP code from the questions since there are millions of them to process, however I found that the regexes, though not perfect, provide a good estimate and are quick to execute.
You could tweak the regexes a bit to fix some things...
- it's currently skipping variable names with underscores, and it's matching variable names that start with numbers, which isn't a valid variable name
- the false positives coming from ->execute($whatever) could be fixed by not searching after a > character. I don't think that would create any notable amount of false negatives.
Like, for example, changing:
- it's currently skipping variable names with underscores, and it's matching variable names that start with numbers, which isn't a valid variable name
- the false positives coming from ->execute($whatever) could be fixed by not searching after a > character. I don't think that would create any notable amount of false negatives.
Like, for example, changing:
'/UPDATE\s+.*?\sSET\s.*?\$[a-zA-Z0-9].*?/i'
'/UPDATE\s+.*?\sSET\s[^>]*?\$[a-zA-Z_]/i'[deleted]
That's what happens when you use Regexes to find problems: https://github.com/laurent22/so-sql-injections/blob/7b9d99c0...
Yes, that's a false positive. I guess the results could be tweaked to filter out lines that include `->prepare(...`.
> Using a dynamic table name is pretty standard practice. The reason for that is to allow a table prefix, allowing multiple of the same app within a single database.
This is precisely how PHP programmers think: that MySQL is somehow the gold standard in the databases world.
Table prefixes are just the stupid way of emulating database schemas where it's not supported, which is only MySQL (SQLite doesn't support this either, but it was never meant for web applications anyway).
This is precisely how PHP programmers think: that MySQL is somehow the gold standard in the databases world.
Table prefixes are just the stupid way of emulating database schemas where it's not supported, which is only MySQL (SQLite doesn't support this either, but it was never meant for web applications anyway).
Table prefixes aren't just used in PHP. Ruby/Rails, Grails, etc. And the general idea of dynamic, variable bound table names can be found in almost any language. Schemas are nice, but as you mention, two of the most popularly used databases don't support it.
Edit: And, for what it's worth, the affinity for Mysql in PHP code is most likely because both are there, by default, in shared hosting environments. I don't personally know any PHP devs that find Mysql to be a "gold standard". Most popular PHP apps and frameworks support Postrgres as well.
Edit: And, for what it's worth, the affinity for Mysql in PHP code is most likely because both are there, by default, in shared hosting environments. I don't personally know any PHP devs that find Mysql to be a "gold standard". Most popular PHP apps and frameworks support Postrgres as well.
> Schemas are nice, but as you mention, two of the most popularly used databases don't support it.
Yeah, two databases. One that was designed as an embedded storage engine and was never intended for web applications and the other that historically was probably the shittiest one (silently damaging unicode data, silently damaging foreign keys in some circumstances, silently ignoring CHECK constraints, and using non-ACID data store unless asked otherwise, though the last one was fixed just few years back) and should have died long time ago.
Yeah, two databases. One that was designed as an embedded storage engine and was never intended for web applications and the other that historically was probably the shittiest one (silently damaging unicode data, silently damaging foreign keys in some circumstances, silently ignoring CHECK constraints, and using non-ACID data store unless asked otherwise, though the last one was fixed just few years back) and should have died long time ago.
All true, but I just don't see it supporting your original assertation that PHP devs collectively think a certain way. Support for those two databases, in web applications, is not unique to PHP. I see plenty of golang, node, ruby, python, etc, code that supports them.
[deleted]
[deleted]
Good point, and I don't think it's possible to use prepared statements for table names. I'll remove the regex for this.
What are the common use cases for this scenario?
Table prefixes? Running more than one instance of an application in a single database. You'll find it's supported in most php apps and frameworks. Wordpress, Drupal, Laravel, Magento, etc.
Not just php either. Ruby/Rails Active Record, for example, supports it.
And I imagine dynamic, variable bound table names are probably in every ORM in just about every language.
Not just php either. Ruby/Rails Active Record, for example, supports it.
And I imagine dynamic, variable bound table names are probably in every ORM in just about every language.
Variable table names can also be useful if you have poor man's partitioning – separate tables with historical data , like "logs_201610", "logs_201611", "logs_201612", etc.
Another scenario I just thought of: your schema is more or less dynamic. For example, Drupal lets developers/site administrators create entities with custom fields, each field gets its own DB table, and to load an entity you need to
Another scenario I just thought of: your schema is more or less dynamic. For example, Drupal lets developers/site administrators create entities with custom fields, each field gets its own DB table, and to load an entity you need to
SELECT * FROM node
LEFT OUTER JOIN field_data_field_name_1 ON ..
LEFT OUTER JOIN field_data_field_name_2 ON ..
...StackOverflow accepts edits to posts, if you have the karma for it.
Somebody might enjoy writing a bot to propose updates that demonstrate better practices (like prepared SQL statements) for the 80% of bad examples where this is a pretty trivial manipulation.
Somebody might enjoy writing a bot to propose updates that demonstrate better practices (like prepared SQL statements) for the 80% of bad examples where this is a pretty trivial manipulation.
There's a small group that does its best to flag questions with comments and steer people towards the right way of doing things. Ninja-editing questions to "fix" problems doesn't teach anyone anything.
Does this site link to the relevant questions, though? Because I can't see it.
Not currently, but the data is there so it would be easy to provide a link for each question.
Edit: Done - the date is now a link to the question.
Edit: Done - the date is now a link to the question.
Thanks very much. I know I've been a bit critical about the implementation in these comments, but it's only because I see such value in the concept, and that's much more important, tbf.
I haven't seen any examples (just code snippets), but I wouldn't call all unsanitized queries SQL injection vulnerabilities:
-----
Obviously that doesn't mean this is very bad practise, especially at stackoverflow where you know people will just copy and paste your code.
$sql = "SELECT * FROM inventory_item WHERE product_category_id='$pcid'";
-----
Obviously that doesn't mean this is very bad practise, especially at stackoverflow where you know people will just copy and paste your code.
yeah, unless the snippet also includes a definition for variable $pcid, there is no proof that the code is actually vulnerable - using such simplistic logic, the stats are likely hugely inflated
but i don't agree that it should be the poster's responsibility to show their sanitation logic to prevent people from blindly copying-and-pasting their code, if it's not relevant to their question
but i don't agree that it should be the poster's responsibility to show their sanitation logic to prevent people from blindly copying-and-pasting their code, if it's not relevant to their question
These are the people we don't hire or if we communicate will with and they seem teachable this is always are first test and step for teaching. I use to be suprised how many college grads come out with a blank face when you ask them about string cleaning or binding and MOST have worked some small internship at a business which means they created a CRUD with plenty of sql injection working for cheap instead of learning.
Side story had to yell at a new employee this year for obsessively posting to stack queries like this instead of asking the senior devs. I've asked 5 stackoverflow questions over 6 years and answered 61..if your asking 3 a day you need to rethink things.
Side story had to yell at a new employee this year for obsessively posting to stack queries like this instead of asking the senior devs. I've asked 5 stackoverflow questions over 6 years and answered 61..if your asking 3 a day you need to rethink things.
What I would also find interesting is to do the same exercise on the top responses.
In principle, it seems like this could be mitigated by some changes to the language. Has anyone tried this? E.g.:
Strings have a dirty flag. Literal strings are initially clean, CGI variables are dirty, argv and file/socket input might be configurable.
Function parameters can be declared evaluated. Passing a dirty string as an evaluated argument fails with extreme prejudice. The database primitives enforce this. Maybe sockets have an evaluated flag too, and only clean strings can be written to an evaluated socket.
The string primitives propagate dirty flags: clean and dirty concatenate to dirty, and so on.
There is an obfuscated way for pdo->prepare to make dirty strings clean, and a special circle of hell for anyone who mentions that in user documentation.
Strings have a dirty flag. Literal strings are initially clean, CGI variables are dirty, argv and file/socket input might be configurable.
Function parameters can be declared evaluated. Passing a dirty string as an evaluated argument fails with extreme prejudice. The database primitives enforce this. Maybe sockets have an evaluated flag too, and only clean strings can be written to an evaluated socket.
The string primitives propagate dirty flags: clean and dirty concatenate to dirty, and so on.
There is an obfuscated way for pdo->prepare to make dirty strings clean, and a special circle of hell for anyone who mentions that in user documentation.
Perl has such a feature, it's called "taint mode" [1], it marks all variables receiving external data as "tainted" and disallows their usage in sensitive contexts.
PHP has an inactive proposal to implement it as well [2].
[1] http://perldoc.perl.org/perlsec.html#Taint-mode
[2] https://wiki.php.net/rfc/taint
PHP has an inactive proposal to implement it as well [2].
[1] http://perldoc.perl.org/perlsec.html#Taint-mode
[2] https://wiki.php.net/rfc/taint
At a previous job we used custom patches for the PHP interpreter which implemented just such a "dirty flag", among other things.
It would also die if you sanitised a clean string, since that indicates that the sanitising logic is dodgy.
It would also die if you sanitised a clean string, since that indicates that the sanitising logic is dodgy.
I think it's even more shocking they appear in answers. But then again this does only show they don't use prepared statements. You can't tell anything about vulnerabilities.
Perhaps this is natural? That is, you're looking for help so you're likely to have other flaws in your tool box.
Certainly, given the importance, something can be done to prevent relying on the human element to get this right every time. We're how many years into programming and SQL injection is still very real? So much for progress?
Certainly, given the importance, something can be done to prevent relying on the human element to get this right every time. We're how many years into programming and SQL injection is still very real? So much for progress?
As a regular user of SO, I'm often amazed at the number of SQL injection vulnerabilities in PHP questions, I had the feeling it was almost in all of them (turns out it's closer to 50% but indeed that's still huge in 2016!). And usually these questions aren't even about the blatant security issue but some minor unrelated problem.
It's not natural. It's unique to PHP.
Python, Perl, Ruby, Java, Node, they all have respect for best practices and libraries that make coding SQL queries safely quite easy and pleasant.
PHP is a whole different beast. People are allergic to package managers, to using "third party code", to using frameworks, even to reading documentation. At times it's amazing how aggressively backwards it can be.
I'm not trying to characterize the entire PHP community, there are large parts of it with people trying hard to do the right thing, but there's also this vast wasteland of incompetent people publishing tutorials that are so dangerously wrong with even more people willing to defend these tutorials as being educational and useful.
Progress means learning from your mistakes, but some would prefer everyone repeat them, painfully if necessary, to promote "education".
Python, Perl, Ruby, Java, Node, they all have respect for best practices and libraries that make coding SQL queries safely quite easy and pleasant.
PHP is a whole different beast. People are allergic to package managers, to using "third party code", to using frameworks, even to reading documentation. At times it's amazing how aggressively backwards it can be.
I'm not trying to characterize the entire PHP community, there are large parts of it with people trying hard to do the right thing, but there's also this vast wasteland of incompetent people publishing tutorials that are so dangerously wrong with even more people willing to defend these tutorials as being educational and useful.
Progress means learning from your mistakes, but some would prefer everyone repeat them, painfully if necessary, to promote "education".
PHP has had the PDO extension since version 5.1, released in 2005. Blaming the language for SQL injection hasn't been a valid excuse for over a decade.
The only problem with PHP is that it has the easiest barrier to entry - or perhaps more accurately, no barrier to entry whatsoever. The kind of developer you're describing would never have written a line of code if PHP didn't exist. Python, Perl, Ruby, and Java just wouldn't have been an entry point into programming for them. Node is different, as like PHP, it is accessible to frontend developers who want to branch off into backend as an "addon" rather than as a primary endeavor.
The only problem with PHP is that it has the easiest barrier to entry - or perhaps more accurately, no barrier to entry whatsoever. The kind of developer you're describing would never have written a line of code if PHP didn't exist. Python, Perl, Ruby, and Java just wouldn't have been an entry point into programming for them. Node is different, as like PHP, it is accessible to frontend developers who want to branch off into backend as an "addon" rather than as a primary endeavor.
PHP the language and PHP the ecosystem are different but inseparable. Although technically PHP has had PDO since 5.1 the community is extremely, frustratingly slow to adopt new things.
I remember considerable resistance on the part of the community to the very idea of object-oriented problem, something that massively limits uptake on things like PDO and is the driving reason behind mysqli having a bizarre mixed mode, OO or procedural based on preference.
Likewise, PHP tutorials, references, and guides kept pushing the absolutely garbage mysql_query interface on people. I don't know how many books published post-PDO insisted on using this approach because it was "easier" or whatever, and some books don't even touch on SQL injection bugs, it's like they don't exist, even while the examples lack escaping of any sort and would crash on certain kinds of input.
> The kind of developer you're describing would never have written a line of code if PHP didn't exist.
Bullshit. These are people that want to learn to program, and as they look around they see PHP is popular, it's easy to get support for, and hosting it is easy. It's got a lot of positive points.
I've seen people with zero technical experience pick up Ruby and build a Rails app inside of months, they get productive very quickly. Same could be said for Node with the right mentoring, though the async stuff is a little harder to get lined up. Client-side apps aren't all that bad, though.
PHP has a lot of good things going for it, but too much of the community is absolutely in the dark about it. There's almost zero outreach being done.
I remember considerable resistance on the part of the community to the very idea of object-oriented problem, something that massively limits uptake on things like PDO and is the driving reason behind mysqli having a bizarre mixed mode, OO or procedural based on preference.
Likewise, PHP tutorials, references, and guides kept pushing the absolutely garbage mysql_query interface on people. I don't know how many books published post-PDO insisted on using this approach because it was "easier" or whatever, and some books don't even touch on SQL injection bugs, it's like they don't exist, even while the examples lack escaping of any sort and would crash on certain kinds of input.
> The kind of developer you're describing would never have written a line of code if PHP didn't exist.
Bullshit. These are people that want to learn to program, and as they look around they see PHP is popular, it's easy to get support for, and hosting it is easy. It's got a lot of positive points.
I've seen people with zero technical experience pick up Ruby and build a Rails app inside of months, they get productive very quickly. Same could be said for Node with the right mentoring, though the async stuff is a little harder to get lined up. Client-side apps aren't all that bad, though.
PHP has a lot of good things going for it, but too much of the community is absolutely in the dark about it. There's almost zero outreach being done.
It sounds like a case of more is less. That is, there's a tipping point - that's not really PHP's fault - where quantity of participants makes quality close to impossible. Yes, there is quality PHP usage but it's rare, if only because the masses create so much noise. Look at WordPress for example.
WordPress is one of the worst examples you could ever point a new PHP developer to. A friend once pointed me to Automattic as an entity to work for (on WordPress) as a nomad developer. I took one look at wp-login.php in the root directory and after 30 seconds of reading, knew I could never take the people that work on that codebase seriously.
The first line of the file is "require( dirname(__FILE__) . '/wp-load.php' );". a) "require" is a statement, not a function; it should not have parentheses around it at all. b) Oh. my. god. Their coding standard require spaces inside parentheses; I have never seen this in any language, even from legacy codebases or esoteric developers - it's incomprehensible that some senior developer managed to enforce this on the entire company. c) Oh, mixed PHP class and HTML; a 1000 line file for a login script. RUN! SERIOUSLY, RUN AWAY!
After this many years, backwards compatibility is not an excuse. WordPress should be avoided at all costs, unless it is the only way you believe you can build and sustain your business. If you have the choice to take any other avenue, take it. The developers working there have no clue, and/or are content with a status quo that is 20 years behind acceptable standards.
The first line of the file is "require( dirname(__FILE__) . '/wp-load.php' );". a) "require" is a statement, not a function; it should not have parentheses around it at all. b) Oh. my. god. Their coding standard require spaces inside parentheses; I have never seen this in any language, even from legacy codebases or esoteric developers - it's incomprehensible that some senior developer managed to enforce this on the entire company. c) Oh, mixed PHP class and HTML; a 1000 line file for a login script. RUN! SERIOUSLY, RUN AWAY!
After this many years, backwards compatibility is not an excuse. WordPress should be avoided at all costs, unless it is the only way you believe you can build and sustain your business. If you have the choice to take any other avenue, take it. The developers working there have no clue, and/or are content with a status quo that is 20 years behind acceptable standards.
To be clear, I wasn't saying WordPress was a positive example. In fact, as you pointed out, quite the opposite. It's been drinking its own Kool Aid for so long that the nOObs don't dare ask questions, and The Elites are too comfortable to progress.
Yeah. You're right. Ugly.
Yeah. You're right. Ugly.
I don't disagree. But that's kinda embodied in what I meant by "natural" in the broad sense. I hear ya.
Maybe a better word is "endemic".
> Perhaps this is natural? That is, you're looking for help so you're likely to have other flaws in your tool box.
Can you explain what you mean by that? The way I'm reading that you seem to imply "looking for help" means that they have issues all over.
Can you explain what you mean by that? The way I'm reading that you seem to imply "looking for help" means that they have issues all over.
You're correct. But let me clarify. I'm not judging. We all have flaws, technical or otherwise. My point was to say, the universe of the sample has a bias of sorts. Kinda like, I have a dog. Well of course it barks.
That's all. Make sense? Sorry?
That's all. Make sense? Sorry?
I'm afraid that didn't clarify anything for me.
> the universe of the sample has a bias of sorts
OK, what?
> I have a dog. Well of course it barks.
I'm with you there. Dogs bark. There's a correlation between the animal being a dog and it barking. How does that relate to your original post?
I have plenty of flaws and gaps in my knowledge and I use SO because there are plenty of great answers on there. Just like there's wonderful threads like these: http://security.stackexchange.com/questions/20803/how-does-s... on other parts of that same network of sites.
I don't see how using SO, or any source of information on a topic to help you solve a problem implies or even correlates to not being a capable person in that or other areas in general.
> the universe of the sample has a bias of sorts
OK, what?
> I have a dog. Well of course it barks.
I'm with you there. Dogs bark. There's a correlation between the animal being a dog and it barking. How does that relate to your original post?
I have plenty of flaws and gaps in my knowledge and I use SO because there are plenty of great answers on there. Just like there's wonderful threads like these: http://security.stackexchange.com/questions/20803/how-does-s... on other parts of that same network of sites.
I don't see how using SO, or any source of information on a topic to help you solve a problem implies or even correlates to not being a capable person in that or other areas in general.
You're asking questions on a forum because you're stuck. You don't know. You might not even know what you don't know.
You're disheveled. You're shirt isn't tucked in. So you're not going to know you're fly is down as well.
The basic question is your shirt. The SQLi issue is your fly.
Put another way again :) Is it reasonabke to expect someone struggling with a SQL question to get SQLi concerns right? Fuck me. They can't get the query. Do you really think they're thinking about injection?
There! Any better? That's all I got. :)
You're disheveled. You're shirt isn't tucked in. So you're not going to know you're fly is down as well.
The basic question is your shirt. The SQLi issue is your fly.
Put another way again :) Is it reasonabke to expect someone struggling with a SQL question to get SQLi concerns right? Fuck me. They can't get the query. Do you really think they're thinking about injection?
There! Any better? That's all I got. :)
solution: ci with http://stackoverflow.com/questions/4863969/php-source-code-a...
It shows only bad practices of PHP and SQL. SO question were asked probably by novice programmers or amateurs. I'm a PHP developer. I use PDO or other data binding ORM/lib (ex. Eloquent ORM).
Next step could be to auto-edit the answers with proper SQL queries? Most of them look pretty simple to fix
!!! PHP MySQL extension doesn't support placeholders so everyone has to escape params before mysql_query and then put escaped params inside query string.
Of course if there's used mysqli or PDO then need to use placeholders but in other case - there is actually no choice for developer.
So I'd rather not call most of those - "vulnerabilities".
I think it's very natural that code in questions is vulnerable to SQL injections as it should be a stripped down example demonstrating the issue that the poster is dealing with, not necessarily a copy-paste of code that's in production.
babyrainbow(2)
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.