Updating Procedural to PDO Legacy PHP Code - approach to addslashes() in the Database

Security and PHP
#1

I am looking for advice and experiences on the best approach to move an old php 5.4 procedural project to a 7.x OOP with PDO.

I am comfortable with having to completely re-write (as I think this is really the only option) I am however looking for some pointers on how to approach an existing production database that has thousands of line entries across hundreds of columns on approximately 50 tables and all entries have at some stage been sanitised with addslashes() - I am clear that addslashes() is redundant (along with some other old approaches to escaping strings) however what is the best approach for retrieving previously escaped data that has been saved in the database with slashes?
Your thoughts and experiences will be greatly appreciated.

#2

Do you actually have backslashes in your database where you don’t expect them? If addslashes has been used “correctly”, the slashes are just used to escape values while they are stored - the slashes shouldn’t have made it into your database.

#3

Firstly thanks for your quick response.

Yes there are back slashes in the database.
I was expecting them - all the corresponding queries to read the data have the variables wrapped in stripslashes() as a result the slashes are not rendered on the client side.

As my OP states it’s legacy code, while it’s running on 5.4 it was originally written back in 4.x and never maintained.

All database queries are also written using the deprecated mysql functions (i.e. mysql_query etc.).
Cheers and thanks in advance.

#4

Before starting a codebase rewrite, I’d get the database in shape. First thing, get the current codebase to stop using stripslashes and addslashes - just use the proper escaping tools instead. DON’T MAKE ANY OTHER CHANGES YET - this is a nervy enough thing to do on its own, you don’t want anything else to keep an eye on. Then write a one off script that can correct the information stored in the database. Test this exhaustively against a backup of your db.

Once you have both of these ready to go, you need to schedule some brief downtime while you run the script against your live db and deploy your new addslashes-free codebase.

This will give you a clean database, and prevent the data being messed up with slashes again.

As an aside, this is how I’d suggest you do the entire migration to PHP 7; lots of refactorings, kept as small as possible. A ground up rewrite is a risky proposition. Joel Spolsky has more to say on it.

1 Like
#5

Thanks,
Your advice is greatly appreciated.
The comments from the article also make perfect sense.
I am also partly responsible for some of the unruly legacy code mess as I spent a lot of time bug fixing while also learning PHP so many hours of breaking an fixing has already occurred.
I will definitely fix the escaping issue first then take baby steps. I was planning on only refactoring one script at a time, bench test, then move to production.
Once again really appreciate the advice.
Cheers

#6

Not quite. First thing… Back up everything!

If you have time, a good read is “Modernizing Legacy Applications In PHP” by Paul M. Jones. The book takes you step by step to refactoring old Php applications.

1 Like
#7

The migration sections in the php.net appendices list the backward incompatible changes, removed, and deprecated features that you must update to get your code to even work and to future-proof your code - PHP: Appendices - Manual

Since you must update the database extension, from the now removed mysql extension, you will need to address protecting queries from sql special characters in data values breaking the sql query syntax, which is how sql injection is accomplished, and having error handling for the database statements that can fail - connection, query, prepare, and execute. Prepared queries provide protection, for all data types, not just strings, and actually simplifies the sql query syntax (all the quotes, dots, and {} used to get php variables into the sql statement go away), and eliminates all of the php code to try and make each different data type safe. You would also use implicit binding (supply an array of values to the execute() call) and for error handling use exceptions for database statements and only catch and handle the exception in your code for user recoverable errors, such as when inserting/updating duplicate or out of range user submitted values. For all other error conditions, just let php catch and handle any database exception.

Doing the above will eliminate most of the existing database implementation logic, so that you don’t need to update it, you can just delete it. All you need to do is trim, then validate input data to insure it meets the needs of your application. The prepared query handles security and exceptions handle errors.

1 Like
#8

One other thing you might consider is moving away from MySQL query system and use PDO instead. It is the way that most new systems access the database. Here is a link about it: PDO

I agree 100% with Benanamen ! ! ! Backup up everything, the database, PHP, HTML, CSS, everything first!

1 Like
#9

Thanks for the suggested resource I will check it out.
Backups are king!
I have been running daily back ups (all code and database) for as long as I can remember and they have got me out of the you know what a number of times.
I also habitually run a DB back up on the production DB before any code change is pushed to the production side (I am paranoid) however yes I think this bit of advice can never be over stated.

#10

Hi @ErnieAlex yup going PDO was the reason for the post (it’s actually in the thread title :wink: ) I have been prepping myself for the procedural MYSQL and PHP to PDO and OOP for a while (12 months of reading) however all the texts I have been reading have been how to write code using this syntax and don’t cover refactoring (for example “PHP Master: Write Cutting Edge Code 1st edition by Shafik, Davey, Mitchell, Lorna, Turland, Matthew” among a few others).
I originally took this project on as a challenge - I am not an IT professional and in my high school days a Commodore 64 was a high end home PC (yep I am way old :grinning: ) anyhow, really appreciate everyone’s input Stack Overflow appears more code problem solving centric - and this forum seems more open to having a discussion and putting out ideas in a respectful manner (I have used SO for a while with great success but glad I posted this question here).

#11

Thanks for the suggestion - I will definitely have time to read it, I have to fly for 4 1/2 hours every second week for work so this will will now be added to my my tablet to keep me sane on those flights :+1: :+1:
Cheers

#12

Well, I post a lot of StackOverflow links here if they have a good solution for the problem.
I was involved BEFORE the COM64 was in use. Therefore, I understand your comments. But, I must say that SO and this site and GitHub and many many others have fantastic solutions to all thing programming…

Now with that said, I personally prefer procedural to object orientated programming. But, if you are creating a huge system, OO is probably the way to go. You did not talk at all about your site, your data, or the current version of your site. You have not told us what the old site is and what you hope to do. Therefore this is not really a “Security and PHP” type of thread.

We can help you in many ways, but, you might not read so much as coding is the way to go! Code, my friend and see what happens. Then, we can discuss your errors and how to fix them!

I suggest telling us a bit of what you want to upgrade and we can help…

1 Like
Sponsor our Newsletter | Privacy Policy | Terms of Service