A C++ Challenge | Hacker News Reader

A C++ Challenge

chargen.matasano.com

33 pointswheels24 comments

I didn't post this to HN, once again the audience for this is security people, not C++ neatfreaks, but if you want to make a go at it, remember the goal is "find the vulnerability in this code that will allow you to take over the program". It's not an abstract problem.

Fun challenge! I wonder when the contest closes? I'd like to blog about my solution. Hopefully my email to matasano doesn't get eaten by a spam filter :/

The code is clearly written by someone who knows C first. If I was given that to review I would reject it.

Using NULL, inconsistent indenting, putting int i outside the for loop, struct's that should be a class and a class that has no reason being a class. Not to mention variables like 'o'* and a class named Obj (woo saved 3 letters and named it the most generic thing I could think of). one struct starts with a _ the other does not... using printf rather then cout. Bad variable naming in general.

And this is all before even reporting the real issues which from the looks of it are more with the c code then the c++ code.

* o does not stand for object, but objects making it an even worse sin

Isn't that out of scope of the challenge? It's a security challenge -- you've got a project that's not created by yourself and you can't assume that the programmer was sane either.

Isn't that what security researchers sometimes have to face -- horrid code that's used by thousands of people anyways? They're not there to judge whether it's nice and properly indented or not, only if it's exploitable...

Also, in C++ NULL == 0, it doesn't matter if you use the defined value or not.

Yes, plenty of development teams are populated by programmers that treat C++ as "C with basic_string and vector". Chris' example here is a distillate of some actual code he reviewed.

And that's OK. The high-level OOP stuff is there if you need/want it, but unlike Java, no one is trying to force that mindset down your throat. There is nothing wrong with C-like C++ code. Nothing at all.

    There is nothing wrong with C-like C++ code. Nothing at all.

On the other hand, recognising that C and C++ treat 'unsigned' differently, and that certain C code may be wrong C++ code... A C/C++ coder who does not recognise that C++/C have different behaviour in certain cases is not much of a coder. (Hm, I believe that's actually one of the rare occasions when saying C/C++ is correct.)

I assume that's the bug in this code - it's neither idiomatic C++, nor is it C, but written in a manner that brings out the neatfreak in me.

Chris and Thomas: something like coverity should catch this particular bug without trouble. Have you guys verified this?

On the topic of NULL the Stroustrup answer: http://www.research.att.com/~bs/bs_faq2.html#null

Really it is just another hint that this is made by a C programmer.

This code has a vulnerability that is only present because it is C++, not C. Find it, and then talk about the hygiene.

Hard to read code hides bugs and issues. The challenge is about finding hard c++ bugs, not about being able to read bad code. If the developer you are interviewing can't find the bug you wont know if it is because of your horrible code or if he didn't know about that aspect of c++.

Fix the hygiene and then lets talk about the actual code

No. We didn't write the example. We're not going to clean it up. Find the bug in this messy code, or (allegorically) ship code that attackers can take over and explain why you did it to your customers.

Now you're starting to see what a nightmare these problems are for developers.

I'm starting to see why I don't use intrinsically unsafe languages like C and C++.

> Find the bug in this messy code, or (allegorically) ship code

, or rewrite this code to use proper checking of untrusted data. I don't know the specifics of your code audits, but the only verdict for the code like this should be to redo it.

If the client is uncooperative or if the reviewer has spare time, then, yeah, try and find the actual exploit. Perhaps, if it is an interview question, it's understandable too. Other than that I don't see a clear reason for going this deep into analyzing dirty code. It may or may not contain an exploitable bug, so just clean it up and move on.

I think you are really missing the point of this exercise...

If the obscurity of the codes impedes your capability to see the bug, then refactor it so that obstacle is removed. I think cleaning up code so bugs become clear is a skill that one may consider a prerequisite for this challenge.

Slightly off topic, but I think its always wise to check argc before assuming argv[0] is populated. Its convention only that argv[0] is the name of the program. Got me once when using execve.

if(argv[1] == NULL)

I'm not smart enough to know what the problem is. But you know what - I don't need to be smart enough. If a problem is so obscure that I am unable to discover it through analysis, either because I lack the knowledge or the skill, then I should have another skill : be able to isolate the problem.

Once I know where the problem is coming from, I can change the affected routines in such a manner that they don't cause the same error.

If I faced such a problem in the real world, it would take too long to really understand the bug. It's much faster to isolate the bug and then force a whole big area of the code out and rewrite it in a different and simpler manner that would not cause such a bug.

You can't isolate this problem, because it only occurs when you have an adversary.

Exactly. That's the problem with security exploits. They are typically edge cases, but whereas the typical edge case affects only a tiny subset of users who accidentally run into it, a security exploit edge case is purposely sought out by malicious users and can potentially affect your company and every other user very negatively. It's the difference between a gas-powered range that has a slightly uneven heat across the burners and a gas-powered range that can be re-programmed through the internet to fill your house with gas that is then set on fire.

Wonderfully entertaining analogy.

I think "wait til it happens and then workaround it" is a pretty bad strategy - by the time you find out about the problem (if you even do), it might have cost you huge amounts of mony, lost business, customer's data, etc, etc.

If you (or your coworkers) wrote the bug once, and you don't make an effort to understand what the bug is, how do you know you won't write it again somewhere else?

I think an attitude of trying to leap over gaps in your knowledge is really destructive. Where do you draw the line at "this is too much trouble to understand, I just won't bother?" It's our job to understand the machine to the best of our ability.

I'd argue that the integer overflow inside new implementation is a compiler bug.

The problem is you're using C/C++ and dealing with pointers directly. What do I win?

Reply on news.ycombinator.com