Generic REQUEST addslashes

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Generic REQUEST addslashes

Post by alex.barylski »

When I say REQUEST I mean all GPC variables...regardless of how you access them...

I know it's not ideal but outside of the security hole that Chris Shiflett(sp?) hilited with using *just* addslashes() on mySql databases...what problems can arise from just using a generic addslashes on GPC arrays???

Strictly security not filtering - I don't care about illegal characters getting in there... :P as long as they can't be used maliciously...

Cheers :)
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

Why would you want to have addslashes applied to all the GPC data? I simply don't see how it would be useful (actually, the exact opposite was true because i found it highly inconvenient having to deal with data that was addslashed...)
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

I don't see it as useful. I would never personally use this approach, but I've been given a source tree and asked to work out some issues and seen this approach being used.

I would love to rewrite the entire source base, but thats not likely...however is generic checks like the above are *practically* insecure (ie: Chris's article although accurate is not a practical concern for my situation).
User avatar
dbevfat
Forum Contributor
Posts: 126
Joined: Tue Jun 28, 2005 2:47 pm
Location: Ljubljana, Slovenia

Post by dbevfat »

Well, if you use a database like MSSQL, where backslash is not the escape character, then addslashes() is of no use -- you have a wide open sql injection security hole.

Also, you're not immune to any attacks that are not affected by escaped/unescaped quote characters, like quoteless sql injections, file including, XSS attacks and such.

Apart from that, I can't think of any issues.
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

What you'll want to do is add a magic_quotes de-emulation layer that runs stripslashes on all incoming data so that you can treat as regular stuff.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

dbevfat wrote:Well, if you use a database like MSSQL, where backslash is not the escape character, then addslashes() is of no use -- you have a wide open sql injection security hole.

Also, you're not immune to any attacks that are not affected by escaped/unescaped quote characters, like quoteless sql injections, file including, XSS attacks and such.

Apart from that, I can't think of any issues.
Hmmm...I'll have to keep that in mind...thanks :)
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Adding slashes to all GPC has the same effect as magic_quotes_gpc - it's nothing more than an annoyance and a misled security measure which has little to no benefits. What matters is if this is being depending on to add security? How is it impacted by having magic_quotes_gpc on and off? Are slashed values being passed directly to the Database without reversal and correct escaping?

All fairly basic security questions. If necessary you might look into setting up some functional tests for a few vulnerabilities, run them, and see what sort of results drop out. A functional test framework of this kind would help pinpoint issues.

I'd add something AC mentioned - a magic quotes removal layer. Use it as a buffer between the $_GET/$_POST variables and any code you use/change. You'll need to do a typical stripslashes() for any manual addslashes() usage, and an additional stripslashes() is magic_quotes is enabled (assuming that has an impact). Then you can pretty much batton down your new code, and any changes you make elsewhere without breaking the rest of the application.

You should be using typical security practice on the newer stuff you intend adding. The rest of the application is likely a lost cause since it's probably been spaghettied over time as security issues arose (presumably).
Post Reply