I liked it too then, but it is a language from another decade, with many design flaws (no concurrency, arguments byref by default, inconsistent syntax (like when to use Set or 0-based vs 1-based collections)) and that doesn't really work as a modern language.
And I did my fair share of php too, that was the only way I would develop a website for many years.
But I think the similarity with php is not so much in the quality of the language, but rather that it is an old language, which strength (easily accessible to beginners) also makes its weakness (too many beginners who never moved on or improved their coding style, I personally know "professional" php developers who still do not know what a class is, let alone a sql injection vulnerability). And it becomes unkillable just because of the cheer number of developers to who it is the only known way to write code.
It has everything to do with Php and the attitude that goes with it.
Php puts up the appearance that web development is child's play . That in itself is not bad. But what makes it bad is that it provide absolutely no safety by default, in how it does that. This is true with the stuff in the language and outside of it (Documentation, community etc..).
and you definitly now that if you concat the id with the SQL that it actually is not a vulnerability. Nobody can inject arbitary SQL into the Long/Int variable.
Totally, you can have some safety via, err, type safety. In the links I posted though (which I now realise match enough files that everyone must see a different results page), I was seeing a tonne of string concatenation on e.g. username fields.
The interesting bit for me is that despite pretty much well every C# SQL example out there using parameterised queries, people still find a way to come up with their own broken alternatives.
I have been told that I did not understood agile/scrum rules because during code reviews of django/pylons frameworks I was spotting the SQL and shell injections. It seems agile is the new death of problem solving by suffocating with hugs of being nice. But believe me: there is no way a 150k$/year coder will take nicely a 50k$/year coder says : there is an obvious security flaw coming from your usual coding practice.
Apparently the way in python communities to avoid injection is security by fight club rules.
1st rules you do not talk of SQL injections
2nd rules you take an ORM that supports sanitizing of arguments, but you do not use it.
I have been profoundly chocked at how my experience of python, the same as my experience of PHP is impacted by the social pressure of the «norms».
People being pissed at my remarks always then pointed at me not using integrally PEP8/pylint/flakes (even though I use pylint to catch only potential errors).
Something is rotten when programming is not about designing/coding/correcting anymore but just about following fashions and trends. But that is the way, I guess, some remains the king of the hill and can buy their expensive cars.
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.
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.
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.
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.
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
> 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.
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.
> Yes, it's unnecessary, but it doesn't cause a problem for MySQL.
It's not necessary in any DB, SQL specifies coercion of any value when the expected data type is unambiguous (and in a bunch of other cases, SQL is pretty weakly typed all in all).
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.
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.
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.
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"
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'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.
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.
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.
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
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?
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.
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?
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
I just did the same :) It flags, for example, the following as an SQL injection:
query("DELETE FROM foo WHERE id=1");$id=2;
In other words, it's flagging any line that begins with "DELETE ... FROM" and then later has a variable. Now, that might be a fairly strong correlation, but it's definitely far from perfect.
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.
... 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.
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.
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
SELECT * FROM node
LEFT OUTER JOIN field_data_field_name_1 ON ..
LEFT OUTER JOIN field_data_field_name_2 ON ..
...
> 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).
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.
> 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.
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.
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.
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.
!!! 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".
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.
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].
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).
114 comments
[ 2.9 ms ] story [ 160 ms ] threadAnd I did my fair share of php too, that was the only way I would develop a website for many years.
But I think the similarity with php is not so much in the quality of the language, but rather that it is an old language, which strength (easily accessible to beginners) also makes its weakness (too many beginners who never moved on or improved their coding style, I personally know "professional" php developers who still do not know what a class is, let alone a sql injection vulnerability). And it becomes unkillable just because of the cheer number of developers to who it is the only known way to write code.
Php puts up the appearance that web development is child's play . That in itself is not bad. But what makes it bad is that it provide absolutely no safety by default, in how it does that. This is true with the stuff in the language and outside of it (Documentation, community etc..).
Doing a quick search on GitHub gives a good number of similar exploits, several per page of results:
C#
https://github.com/search?l=C%23&q=sqlcommand+where+username...
Java
https://github.com/search?l=Java&q=sqlcommand+where+username...
Just look at the sheer inventiveness that goes into doing things The Wrong Way; it's not a language-specific problem.
The interesting bit for me is that despite pretty much well every C# SQL example out there using parameterised queries, people still find a way to come up with their own broken alternatives.
I have been told that I did not understood agile/scrum rules because during code reviews of django/pylons frameworks I was spotting the SQL and shell injections. It seems agile is the new death of problem solving by suffocating with hugs of being nice. But believe me: there is no way a 150k$/year coder will take nicely a 50k$/year coder says : there is an obvious security flaw coming from your usual coding practice.
Apparently the way in python communities to avoid injection is security by fight club rules.
1st rules you do not talk of SQL injections
2nd rules you take an ORM that supports sanitizing of arguments, but you do not use it.
I have been profoundly chocked at how my experience of python, the same as my experience of PHP is impacted by the social pressure of the «norms».
People being pissed at my remarks always then pointed at me not using integrally PEP8/pylint/flakes (even though I use pylint to catch only potential errors).
Something is rotten when programming is not about designing/coding/correcting anymore but just about following fashions and trends. But that is the way, I guess, some remains the king of the hill and can buy their expensive cars.
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
$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.
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.
[1] http://www.freewebmasterhelp.com/tutorials/phpmysql/7
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
That's a completely different issue and not an SQL injection error though.
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.
OTOH it is entirely unnecessary to quote integers (and the escaping function would not quote it before injecting it), so outlook not so good.
I've seen lots of people do that though I don't know their rationale. Maybe they just don't know SQL
It's not necessary in any DB, SQL specifies coercion of any value when the expected data type is unambiguous (and in a bunch of other cases, SQL is pretty weakly typed all in all).
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.
But I think the absolute % is missing the point. What I find interesting is the evolution. It doesn't go down.
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 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.
nobody would expect that
Note that PHP also has a JavaScript-style triple-equals comparison, which does not attempt type conversion and does not exhibit this bizarre behaviour.
This occurs if PHP thinks both strings could be numbers in scientific notation. "0e123" == "00e45"
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.
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.
They should still be fixed, but I believe these bugs are no longer an issue in PHP after 5.3.
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.
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.
This extension was purchased through Magento Connect just a few months ago. I'm glad to hear the situation is improving now.
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.
[0] https://stackoverflow.com/questions/40958763
-----
Obviously that doesn't mean this is very bad practise, especially at stackoverflow where you know people will just copy and paste your code.
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
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?
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.
That's all. Make sense? Sorry?
> 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 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. :)
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".
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.
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.
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.
Yeah. You're right. Ugly.
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.
Edit: Done - the date is now a link to the question.
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.
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.
Edit: You can also escape the scans by using variable names that start with an underscore :) $_dontscanme- 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:
To... 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 :)
So people who use
will not get detected?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.
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
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).
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.
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.
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.
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.
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
It would also die if you sanitised a clean string, since that indicates that the sanitising logic is dodgy.