109

I've been hired by someone to do some small work on a site. It's a site for a large company. It contains very sensitive data, so security is very important. Upon analyzing the code, I've noticed it's filled with security holes - read, lots of PHP files throwing user get/post input directly into mysql requests and system commands.

The problem is, the person who made the site for him is a programmer with family and children who depend on that job. I can't just say: "your site is a script kiddie amusement park. Let me redo it for you and you'll be fine."

What would you do in this situation?

Update:

I followed some good advice here and politely reported to the developer that I've found some possible security flaws on the site. I pointed out the line and said there could be a possible vulnerability for SQL injection attacks there, and asked if he knew about it. He replied: "sure, but I think that to exploit it the attacker should have information on the structure of the database; I have to understand better".

Update 2:

I said that's not always the case and suggested he follows this Stack Overflow question link in order to deal with it properly: How to prevent SQL injection in PHP? He said he would study it and thanked me for telling him before. I guess my part is done, thanks guys.

Community
  • 1
MaiaVictor
  • 5,820
  • 7
  • 27
  • 45
  • 29
    I'd really enjoy a solution that does not involve ruining someone's life. I'd leave this alone but I also know such a security hole could, too, ruin some people life. Complicated. – MaiaVictor Mar 04 '13 at 19:37
  • 2
    No need for that, the problem, and priority, is the security flaws. Don't make this personal and go after somebody else. Fix the site, and train their people so the mistakes aren't repeated. – Eric Hydrick Mar 04 '13 at 19:37
  • You mean, just fix the site without reporting the issue? I could if it was small, but it's 200+ PHP files worth of security flaws to analyze. – MaiaVictor Mar 04 '13 at 19:39
  • 18
    The attacker could use the exploit to get the information on the structure of the database. No SQL injection vulnerability should *ever* be downplayed. – Dave Rager Mar 04 '13 at 20:07
  • 17
    Show him how to exploit some vulnerability without using any knowledge of the database. That will scare the sh*t out of him. – Euphoric Mar 04 '13 at 20:14
  • 3
    "sure, but I think that to exploit it the attacker should have information on the structure of the database;" man, he REALLY doesn't understand SQL injection! – ozz Mar 04 '13 at 20:24
  • 74
    I just want to say good job for looking out for another person/programmer that you don't know. It doesn't make it any less of a terrible thing to ruin their livelihood because they made a mistake and you don't know them, and I commend you for taking it into account. – Steel Mar 04 '13 at 20:28
  • 6
    @user16764 Wouldn't that get OP a reputation of being the contractor who, if brought in, will recommend to your boss to fire you? – rakslice Mar 05 '13 at 02:39
  • 2
    Halfway out the door, "Oh by the way, in like 30 other places I noticed that you're f-" Jokes aside: good call. – Erik Reppen Mar 05 '13 at 04:00
  • 7
    @Rythh I appreciate but am kind of scared by your message. That should be the default behavior. I know US has some cultural differences, but why would somebody act differently? We are all members of the same specie, working together for the same goals. Hurting one hurts the whole. Also, it could be you. That doesn't make sense. – MaiaVictor Mar 05 '13 at 06:58
  • I've not seen it mentioned, but I *STRONGLY* suggest you read the material on OWASP (https://www.owasp.org/index.php/Main_Page) and circulate it around the team as well as look into something like IronBee (https://www.ironbee.com/), an Open Source Web Application Firewall. Full Disclosure, I do some work on IronBee. – Sam Mar 05 '13 at 07:38
  • 8
    @Dokkat The issue is one of balance. From a programmer's perspective, the poor quality programmer with a wife and child has actually endangered the company, and thus the jobs of many employees with wives and children. Also, the issue is often complicated by the emotional issue of "Bad programmer does something that makes my life harder. Now I have to miss out on spending time with my family. They are more important to me than he is. This seems unfair." That's an irrational response, but folks be folks. – deworde Mar 05 '13 at 09:41
  • 2
    You are assuming 1) he's going to be fired; and 2) once he's fired, he will not be able to find another job ever. – Mister Smith Mar 05 '13 at 14:22
  • 5
    @Dokkat it should be the default, but trust me when I say it most certainly isn't. If you had walked in there talking all about the terrible job the previous programmer did and how you could fix it but it would take more time and money, that puts you in quite the good position, and them in a terrible one. As many people on this post have already defended doing exactly that for various reasons, you can see why I would commend you. No need to be humble, you did a good thing, recognize that so you can do it again next time! :) – Steel Mar 05 '13 at 19:42

7 Answers7

114

First and foremost here, the priority is to close the security holes.

If you're working directly with the engineer who wrote this, document everything and give it to that engineer.

If not, tell your employer the security issues are bigger than initially thought and that the site needs a lot of work. Ask to work with the main developer who's on the site, and offer to teach them about PHP security (don't promise to make the person an expert, but do offer to train them in everything you know) so that person can take it over after you're done.

Don't make this a "this guy is bad, fire him" issue. Approach it from the perspective of "Hey, I found some potential bugs that need fixing stat, which seem to be coming from some ignorance/common misconceptions about site security. I'd also like to talk to your development so we can improve your site and hopefully avoid more of these issues in the future."

Eric Hydrick
  • 2,451
  • 3
  • 19
  • 19
  • 1
    Great answers overall. The topic is subjective, so I'll mark yours as it was the most accepted by the community. – MaiaVictor Mar 05 '13 at 07:07
  • 1
    If you are working WITH the engineer, but being paid BY management, should you not report TO management? What if the engineer thanks you, but the moment you leave, destroys the report? – Konerak Mar 06 '13 at 08:28
  • Either tell both, or tell the engineer first, and verify that bugs are being created and tracked in whatever system they use. If bugs aren't created, tell management. – Eric Hydrick Mar 06 '13 at 15:59
  • 2
    I liked this answer better: http://programmers.stackexchange.com/a/189206/28351, because for the employer the priorities are different. First report the security holes, then fix the small bug. – nalply Mar 07 '13 at 19:28
80

There's a difference between ignorance and incompetence. There was a time when you didn't know what SQL injection was either, and there's no reason to believe the original programmer isn't capable of fixing the problems once he is made aware of them.

So tell them. Be specific and objective, and make yourself available to answer questions, provide examples of exploits, and recommendations for fixes. If they still don't get it after that point, the most you can really do is not put any of your own personal information on the site.

Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
20

Your job isn't to redo the site for him. It's to fix the small bug. However, if you've noticed security issues that should be fixed you can bring it up with the site owner and offer insight on what the problem might be.

Don't berate or talk negatively about the original developer or comment on how horrible the code is. Be respectful and professional. You can offer to work with the developer to resolve the issues. Don't try to fix it yourself or offer a solution unless you've been contracted to address the problem. If they follow your advice and you're wrong they could come back on you.

Dave Rager
  • 874
  • 6
  • 9
17

First and foremost - fix the thing they hired you for. If you don't do that, then you'll be perceived as the type of consultant who's interested in making more work for themself, rather than getting the job done.

Along with the fixes, you need to give them a list of the stuff you've noticed that's wrong from a security perspective, and why these things are wrong.

Michael Kohne
  • 10,038
  • 1
  • 36
  • 45
13

It won't do anyone any good to not report the issues. If you had a specific task you were hired for complete it but document other security issues as you see them and report them to the appropriate individual, probably the individual that you are reporting to for the task you were hired for.

This is a situation where strong soft skills are going to come in handy as to handle this with tact will require not putting down the work done by others on the site and not making the developer feel as if you are questioning his talent.

Obviously avoid words like "crap, bad, poor, riddled" when referring to the code/flaws and similar words for the developer that wrote the site.

Rig
  • 1,497
  • 17
  • 21
  • 4
    I would add: make sure the developer is aware of the severity of the flaws and how they can be exploited. Taking the time to launch a controlled 'attack' on a local machine with this developer present could do much to educate him on the problem, which leaves openings for you to suggest ways to harden the code. – Andrew Gray Mar 04 '13 at 19:38
7

In addition to the other answers what you might want to do is point the developer at some resources on how easily SQL injection issues can be exploited, for example sqlmap which is an automated SQL Injection exploitation tool.

What I've found to be effective in demonstrating the seriousness of this kind of issue in the past is showing what can be done with it, so if you run something like that against a dev. copy of the site to show it extracting data etc you might convince them of the seriousness.

Rory McCune
  • 181
  • 5
  • 4
    Be aware this has some risks, in that you can be made to seem like a "hacker". Management people do not necessarily understand terms like "existing vulnerability", "development copy" and "white hat security analyst" – deworde Mar 06 '13 at 10:14
0

First and only; Management does not want to hear about problems. I got fired from the Office of Personnel Management (security clearances for the white house) because I pointed out how insecure their system was. That was awhile back, but management attitudes have not changed.

Address the problem with the developer, via email so you have a trail, then walk or run away. When they do eventually have a problem, as a contractor, they will try to blame you, regardless of having any involvement even remotely connected to the problem.

Having a problem as fundamental as a SQL injection, indicates they were cheap when they initially developed the system, and odds are they are at best cheap now. Get what you can from them while they are still in business, but seek business development elsewhere.

joe
  • 19
  • 1
  • 3
    "Management does not want to hear about problems" -- add some reasoning / references to support your assertion (which sounds plausible to me but this doesn't really matter) and I will revoke downvote – gnat May 11 '13 at 22:19