- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Not TRWTF enough. Where's
my_do_queryandmy_is_numeric?Admin
"high risk change to a stable system"
Nancy works for Equifax, which values stability over security.
Admin
I would just quit that job.
Admin
While the code is awful, the last paragraph is the killer. I hope this isn't a bank.
Admin
It's always amusing when the business has no idea what an actual risk is in the system, and refuses to listen to IT when they say it is. Yet if it was someone outside the company, say an auditor, who saw the vulnerability and said something, then suddenly they listen. Never understood that mindset.
Admin
I guess that they have a formal procedure for any suggested code change to evaluate how much of a risk of causing problems and this is very likely to be able to cause problems, especially if users of the function already found out about its deficiencies and applied correct escaping before calling.
To then add more escaping you might break things
So its stated correctly as a high risk change.
Evaluating existing code is another thing altogether, what value can come out of that ? :P
Admin
Nah, the play nowadays is to report your vulnerability on Twitter to make as big a splash as possible, so the management and executive take it as a PR problem and demand a fix ASAP.
Admin
Indeed it is amusing, but it's quite easy for the PHBs to blame the developers/IT when the database suddenly disappears, even if they have been warned about the risk multiple times....
Admin
It's easier to explain it than you'd think. Say you found a sparkly rock in the middle of the road, and you get it for free, how much do you think it might be worth? Say you enter a shop and buy a diamond for 5000$, how much do you think it might be worth? The diamond is actually worth less than you paid for, since companies withhold a huge stock of them just to make the price skyrockets, if you try to SELL that diamond, you're going to be surprised about its actual worth. Thus people give it value BECAUSE they paid for it: both for signaling status and for rationalizing their purchase. It's what you paid that made it valuable, the actual rock could be the same.
"But Frank" you might say "this is a technical decision, it doesn't follow economic principles!" It shouldn't. But then again, economy is all they know. And Free stuff is worthless or there to push you into spending, as we all know.
Admin
If you like PiU+00F1a Colada stdout(my_escape)
Admin
I don't see the problem. It doubles up any single quotes, which stops the
robert'; DROP TABLE STUDENTS;--exploit. What else do you need?Admin
TRWTF is using PHP for anything.
Admin
At the risk of being whooshed... there is so much that can be done event with more filtered stuff: https://www.exploit-db.com/papers/17934
Admin
I've made two managers, in two different companies, see the problem and demand an urgent change by showing how easy it was to log in to one of their own accounts. However, the same people who implemented the first broken version were assigned to fixing it. You can imagine how bad the "fix" was. It was enough to stop me (at one company an intern still in uni; at the other company a junior dev fresh from uni...) from being able to show how to exploit it, so in their heads it was 100% fixed.
Admin
it's the "an expert is someone from out of town with a briefcase" mentality. it NEVER made sense.
Admin
I want my_redemption to be called after my_escape.
Admin
I actually don't see any examples in there of how you'd perform injection if the only query is in the form of the one presented here, and all single quotes are doubled up by an escape function. Apparently with some databases with unicode character mapping it is possible though, but are PHP strings unicode? Would definitely be interested in seeing an example of a value that this code (terrible as it is) wouldn't handle.
Admin
It's possible the auditor is there as either a legal obligation, or required by a client. In either case, ignoring an audit could directly lead to lawsuits and/or losing customers, and the pressure from the audit is coming from somewhere outside the manager's influence. Conversely, complaints from the IT department won't directly lead to lawsuits or losing customers, and the manager probably feels he/she has enough power to simply ignore them.
Admin
Oh there are still tons of exploits you can do this way, even when they try to just double all the single quotes. For example, what about all the numeric queries?
| SELECT * FROM USERS where id=blah
If they're dumb enough to be just pasting the blah in here, then they're often dumb enough to just be reading the blah from the form or url, and you can replace it with "3 OR 1=1" to get all info for all the users:
| SELECT * FROM USERS WHERE id=3 OR 1=1
Now you need another vuln here which is that it actually shows them to you, but it might use common code to view a single item or multiple items.
There's another giant hole too. Lots of SQL servers let you escape single quotes, like with a backslash.
So now you have:
| SELECT * FROM users WHERE name='foo;
Just escape foo and you have
| SELECT * FROM users WHERE name='''; DROP TABLE users; --'; -- bobby tables in the house
There's lots more you can do, but that should be enough to demonstrate. It's guaranteed that anyone who rolls their own quoting will get it wrong. Just use bound parameters. Or if you MUST, then call the DB's own quoting function, like mysql_real_escape_string()
Admin
Odds are it is a bank, a health company, or anything where security is most important. Because they always have the worst security.
Admin
Hmm... Banks and other financial institutions don't actually have security as their most important metric. We could argue that it should be, but it isn't.
Most important to a system that handles people's money is that it works the same known-right way that it worked previously. Making sure that the processing that didn't get them in financial trouble last week also won't get them in financial trouble this week or next week.
Like it or not - agree with it or not - but that is a huge hurdle to overcome to get something else instilled as the highest priority of the code development managers.
Admin
This is how my_real_escape() is born. And the configuration switch that turns, or not, my_escape into my_real_escape.
Admin
Trivial.
\' OR TRUE;#which then unfolds tobla = '\'' OR TRUE;#'.Admin
I'm just amazed that PHP has no function for this. It has the double_kitchen_sink() function, why not sql escaping?
Admin
But that's changing the query. I specifically asked if there were an example value for THAT query. Nobody's arguing it's an adequate general purpose SQL escape function (and given all modern day SQL databases support parameters there's not even a need for such a thing).
Admin
Holy shit. Dunno how old this code is, but anyone not using PDO or mysqli_ when doing php (use PDO) should be taken behind the shed and given a summary ball-snipping. With rusty, dull scissors.
Admin
But Frank, they only paid the consultant $1k (two hours at $500/hr).
They paid me many times that over the course of the year. My hourly rate is a lot less, but it adds up over time.
It's just that they were already paying me, so they disregard that.
Someone who's been audited, that works for external audits. To a lesser extent, it works for internal audits, as those are primarily to find things before external audits show them. It doesn't explain why they care more about what consultants say.
Admin
I agree that the code is truly awful, but unless you assume that either there's an almost identical function for numbers which doesn't have the quotes (which isn't mentioned here), or that you're using an RDBMS which lets you escape single-quotes with something other than another single-quote (MSSQL uses '' to escape ', so that won't work), I still don't see a way of attacking it. There are a lot of comments held for moderation though, so maybe someone else here has already thought of one.
Admin
It's the same mindset that leads to "we don't have time to fix bugs that are reported internally because we have too many reported by customers."
Admin
That can be flipped on you: You're only looking at the direct security risks. But there are plenty of other risks around as well:
I'm obviously not arguing that security holes shouldn't be fixed, but they're not the only risk and depending where they are in a system, fixing them may well itself be a huge business risk, especially if they're buried deep in the system where anything less than an extremely targeted hack would be extremely unlikely to ever be a problem. My own company has a few holes where the only real attack vector is "an ops engineer goes rogue." Its technically a vulnerability, but not the most concerning one in the world.
(Of course in the story's example, it sounds like its a systemic problem which likely means a host of easily-exploitable APIs exposed to the whole internet, and that's almost certainly a wide enough attack vector to override all of the risks of fixing it.)
Admin
If the dB is MySQL, and NO_BACKSLASH_ESCAPES isn't enabled, it's vulnerable to injection. Otherwise, it's just crap but maybe not dangerous.. depends on the RDBMS, but it's not mentioned here.