5

I know this topic has been covered to death but I would like some feedback from the community regarding security within our web application.

We have standard LAMP stack web app which contains a large number of database queries which are executed using mysqli_query. These queries are not parameterized and at the moment but there is some naive escaping of the inputs using addslashes.

I have been tasked with making this system safer as we will be penetration tested very shortly. The powers above know that parameterized queries are the way to go to make the system safer however they don't want to invest the time and effort into re-writing all the queries in the application and also changing the framework we have to make them all work correctly.

So basically I'm asking what my options are here?

I've run mysqli_real_escape_string over the inputs. I've setup a filter which doesn't allow words like SELECT, WHERE, UNION to be passed in which I guess makes it safer. I know mysqli_query only allows one query to be run at once so there's some security there (from concatenating updates onto the end of of selects).

Do I have any other options here?

Edit: I should probably add that if anyone is able to provide an example of an attack which is completely unavoidable without parameterized queries that would also be helpful. We have a query which looks like this:

SELECT
pl.created
p.LoginName,
pl.username_entered,
pl.ip_address
FROM loginattempts pl
LEFT JOIN people p ON p.PersonnelId = pl.personnel_id
WHERE p.personnelid = $id
AND pl.created > $date1
AND pl.created < $date2

I've substituted a UNION query into the $id UNION SELECT * FROM p WHERE 1 = 1 sort of thing and I can prevent that by not allowing SELECT/UNION but then I'm sure there are countless other types of attack which I can't think of. Can anyone suggest a few more?

Update

I've convinced the powers that be above me that we need to rewrite the queries to parameterized statements. They estimate it will take a few months maybe but it has to be done. Win. I think?

Update2

Unfortunately I've not been able to convince the powers that be that we need to re-write all of our queries to parameterized ones. The strategy we have come up with is to test every input as follows:

If the user supplied input is_int that cast it as so. Same for real numbers. Run mysqli_real_escape_string over the character data. Change all the parameters in the queries to quoted strings i.e.

WHERE staffName = ' . $blah . '

In accordance with this answer we are 100% safe as we are not changing the character set at any time and we are using PHP5.5 with latin1 character set at all times.

Update 3

This question has been marked as a duplicate however in my mind the question is still not followed answered. As per update no.2 we have found some strong opinion that the mysqli_real_escape string function can prevent attacks and is apparently "100% safe". No good counter argument has since been provided (i.e. a demonstration of an attack which can defeat it when used correctly).

Community
  • 1
  • 1
tom808
  • 320
  • 2
  • 11
  • One interesting question somehow related to your problem: http://stackoverflow.com/questions/32477442/sql-injection-protection-with-only-str-replace Basically - check EVERY user input -> check var types, check length, remove 'bad words' as you already did.... – sinisake Oct 02 '15 at 08:38
  • If you are using mysqli in procedural mode, then `mysqli_real_escape_string` is enough I think. I wrote my own database wrapper, what is changing the type (for example, 1 in string to integer), and escape all the strings in the passed array for my method. Or you can use it in OOP mode, and use prepared statements. – vaso123 Oct 02 '15 at 08:39
  • 5
    The Titanic is sinking, how do I stop it with papier maché and sellotape? No matter what you do, you're likely to leave holes by accident. Whether it's something like not properly handling invalid UTF-8 sequences or unexpected ways of including SQL or any number of other possible exploit paths. – Phylogenesis Oct 02 '15 at 08:39
  • No amount of escaping and sanitization is going to stop all injection attacks. Besides, a complex solution is far more likely to introduce exploitable bugs. – Panagiotis Kanavos Oct 15 '15 at 15:28
  • So you completely disagree with [this](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string/12118602#12118602) answer? Apologies for being forward but I need some more concrete evidence that there is a way to get past mysqli_real_escape_string if we follow the practice outlined above. – tom808 Oct 15 '15 at 15:33

2 Answers2

7
  • check every single user input for datatype and where applicabile with regular expressions (golden rule is: never EVER trust user input)
  • use prepared statements
  • seriously: prepared statements :)

it's a lot of work especially if your application is in bad shape (like it seems to be in your case) but it's the best way to have a decent security level

the other way (which i'm advising against) could be virtual patching using mod_security or a WAF to filter out injection attempts but first and foremost: try to write robust applications (virtual patching might seem to be a lazy way to fix things but takes actually a lot of work and testing too and should really only be used on top of an already strong application code)

  • I'm either going to have to test every input and try to get the datatype or I'm going to convince the big man upstairs to invest the time in parameterizing everything. I have no idea how to test for strings though so I guess just have a fixed length? – tom808 Oct 02 '15 at 09:37
  • 1
    it depends on the field itself, i would identify every single one and first of all limit it to valid characters, for example if you have a name and surname you can probably limit to upper and lowercase plus accented, if you have a date field just numbers, you can actually create a validator class (or use one provided with the php framework you are using) and simplify your code, pretty much every framework has a good and tested way to do this and save you a lot of time –  Oct 02 '15 at 09:42
  • 1
    There is a [hacky alternative solution](http://stackoverflow.com/a/12710285/2224584), that is likely to be secure in most cases, but seriously *use prepared statements*. – Scott Arciszewski Oct 02 '15 at 15:47
  • Also add: extensively test all these checks to ensure no new bugs are introduced. Which is why parameterized queries are a better alternative every time – Panagiotis Kanavos Oct 15 '15 at 15:28
5

Do I have any other options here?

No. No external measure, like ones you tried to implement, has been proven to be of any help. Your site is still vulnerable.

I've run mysqli_real_escape_string over the inputs

Congratulations, you just reinvented the notorious magic_quotes feature, that proven to be useless and now expelled from the language.

JFYI, mysqli_real_escape_string has nothing to do with SQL injections at all.

Also, combining it with existing addslashes() call, you are spoiling your data, by doubling number of slashes in it.

I've setup a filter which I guess makes it safer.

It is not. SQL injection is not about adding some words.

Also, this approach is called "Black-listing" it is proven to be essentially unreliable. A black list is essentially incomplete, no matter how many "suggestions" you can get.

I know mysqli_query only allows one query to be run at once so there's some security there

There is not. SQL injection is not about adding another query.

Why did I close this question as a duplicate for "How can I prevent SQL-injection in PHP?"?

Because these questions are mutually exclusive, and cannot coexist on the same site.

If we agree, that the only proper answer is using prepared statements, then a question asks "How can I protect using no prepared statements" makes very little sense.

At the same time, if the OP manages to force us to give the positive answer they desperately wants, it will make the other question obsoleted. Why use prepared statements if everything is all right without them?

Additionally, this particular question is too localized as well. It seeks not insight but excuse. An excuse for nobody but the OP personally only. An excuse that let them to use an approach that proven to be insecure. Although it's up to them, but this renders this question essentially useless for the community.

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 1
    I'm painfully aware that this is a poor approach but I'm looking for either an example of an attack which is unavoidable through this approach (which I can emulate) or some further sanitization I can perform to make the system that 0.0001% safer. – tom808 Oct 02 '15 at 08:48
  • The reason we are so reluctant to move over to parameterized queries is that it will take months to move all our queries over to this method. Think thousands of complex queries which are constructed at run time based on what inputs are provided. We have so far come to the conclusion that the way to get us to being close to safe from injection attack is to use a combination of identifying the numbers and also using mysqli_real_escape_string on the strings. This [answer](http://stackoverflow.com/a/12118602/1261671) has lead us to believe we should be ok. As ircmaxell says we are 100% safe! – tom808 Oct 14 '15 at 08:12
  • 1
    We agree that using prepared statements is the right answer however we do not agree that using prepared statements is the only answer. Based on the answer to the similar question I have linked it claims that it is possible to be 100% safe without them. Perhaps this is better moved into another question/further discussion? – tom808 Oct 16 '15 at 14:03