0

I have a small headache with an old project of mine. I wanted to put back online a legacy version of the website I once managed. The problem is, it was coded with PHP back in 1998+, at time when I had little to no knowledge about security issues (15yo taking first lessons in scripting stuff). When I examine the code now, I can see very little harm that can be possible done since most of the code does basic things without much tampering with vulnerable assets. With one exception, MySQL queries. I have literally thousands of selects, inserts and updates which are wide open for any MySQL injection attempt. The project was big, there are lots of files and lots of code. Even if I search and examine every occurence of mysql_query, I might still miss something. Lots of mess as well. Things like this are all over the place:

        function Mess($ID) {
    $result = mysql_query("select * from table where `id` LIKE '$ID'"); 
}

I was thinking if would there be possibly some easy way to deal with that without spending hours and hours on examining every single MySQL query. Not to mention creating all the PDO structure and trying to intagrate it with this monstrosity. I'm just doing that in my spare time to honor the work lots of people devoted to creating content for this project years ago. So I was thinking about some sort of "general" solution. I was considering php prepend file but what could I possibly insert there to not cripple the incoming variables? I could just kill all GET,POST,COOKIE vars but this would prevent the website from providing content properly. I could disable all but SELECT access rights (I don't need more since this project is not ment to be updated) for the database user and then suppress error messages but that would still leave it open for injections, just without possibility to modify the database. Is someone aware of something I am not, something that would "overrule" the issue? I dont mind if someone will be able to tamper with the variable to view unitended content of that particular DB. There is nothing in that database which is private only (I deleted things like passwords, e-mails etc.). I do mind however the security and intergrity of other data stored on that host.

Avi
  • 129
  • 2
  • 8
  • include a `sanitize_everything.php` file on top, that goes through all possible $_GET, $_POST, $_COOKIE or whatever is used and sanitize accordingly. you need to list all of your variables there and sanitize them one by one if they are `isset` – Sharky May 22 '14 at 06:25
  • if you only leave the `SELECT` access rights, you would only be in danger of showing more information from that query, but not to any tampering with the database. This all depends on the amount of time you want to spend on the project and the data in the DB, but for quick results you could just disable everything but the `SELECT` – Catalin Deaconescu May 22 '14 at 06:40
  • 1
    @Sharky there's no magic sanitizing function that gets a bunch of random values and "secure" them. Security comes with a context, and hence can't be over-generalized (for instance, how does the script know that `$_GET['someKey']` has to be casted to an `integer`? – svvac May 22 '14 at 06:40
  • 2
    @CatalinDeaconescu, that's not the only risk with a SELECT -- anyone can crash your server with a trillion-row join with nothing but SELECT privilege. – Bill Karwin May 22 '14 at 06:51
  • @swordofpain because *"you need to list all of your variables there and sanitize them one by one"* i already made clear its a manual job, one by one. i never said there is a magic button. in my applications, i white-list my inputs, and sanitize each one properly. anything else that may come from input is just not used. for example if `$_POST['id']` is a number, i cast it to integer. Of course there is a sticky note somewhere that lists ALL the variables and what are they used for. – Sharky May 22 '14 at 07:48
  • cant edit my comment above 5-min restriction: correction: *if **i use** `$_POST['id']` **for sending** numbers* – Sharky May 22 '14 at 07:54
  • @Sharky The trick is you can't use the same variable on two different pages to store two different kind of data. Anyway, I just wanted to emphasize the *no magical solution* point. Also, I'm curious about the advantages of the `sanitize_everything` way over just santizing the data used on the current controller/page. – svvac May 22 '14 at 08:16
  • @swordofpain exactly, thats the restriction. you cant use same variable for different types of data if you follow my approach. the `sanitize_everything` way could be fast to implement (just include it on top of every file) **if and only if** a variable is used for a specific type of data. if this is not the case, then its the painstaking *"santizing the data used on the current controller/page."* because it will be for **each** page, ouch. – Sharky May 22 '14 at 08:31
  • @Sharky Even when you add that to an existing project, you'll need to browse the code for every query you make in order to look which parameters you use... But I guess a `grep -r mysql_query /path/to/project` is faster than editing every file :D Thanks for sharing, I never thought of that approach! – svvac May 22 '14 at 08:41

2 Answers2

3

There is no one-size-fits-all solution to SQL injection. If there were, they would build that into every programming language and framework, and then SQL injection would cease to be a problem. They tried that with magic-quotes but it didn't work out and that feature is now deprecated.

See my presentation SQL Injection Myths and Fallacies for more details on methods of protection.

If you want to honor this old project, I suggest taking some screenshots of it and posting them as static images.

Bill, do you have a lecture, based on that presentation?

You can view this as a webinar here (free but requires registration):
http://www.percona.com/webinars/2012-07-25-sql-injection-myths-and-fallacies

And I also presented it at the San Francisco MySQL Meetup:
https://www.youtube.com/watch?v=o4dJ7hdA8fs

Community
  • 1
  • 1
Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • Interesting presentation Bill. I must say I learned few new things from it. Thanks for the link. That being said, posting static images is out of the question due to size of the website. That is lots of content from several years. – Avi May 22 '14 at 07:08
  • Nice presentation! It helped me understand the problem (and solutions) even more. – Nurp May 22 '14 at 18:48
  • @user3663552, if you need the app to be live on the web, then you'll have to get cracking on reviewing and fixing all the unsafe SQL queries. By the time you're done, you'll have so much practice coding all the various fixes for SQL injection that you'll never tolerate writing unsafe code again. Sounds like a win to me! – Bill Karwin May 22 '14 at 19:47
  • After reviewing my options I settled with filtering out all special characters and certain mysql statements (in case of some stray queries with user input not placed in quotations) out of all possible user inputs. Also narrowing privileges to select only, just in case. This would cripple most of websites but for my purposes it's OK, seems like 99% of content works fine. And sure, learning how to code with proper standards is always a good thing. But most of us started at some point from creating greenhorn code. The thing I'm talking about was my kindergarten. + Again, thank you for good links. – Avi May 23 '14 at 15:58
-2

To lower the risk, take an editor that can do regular expression based search and replace through all of your files at once (netbeans or notpad++ for example) and run a search on

(mysql_[a-z]+\("[^$"]+)(\$[a-z][a-zA-Z0-9]*)

and replace it with

$1" . mysql_real_escape\($2\) . "

This should escape all of your queries. Make a backup before you try it!!! ;-)

user3660594
  • 1