A C++ Challenge

http://chargen.matasano.com/chargen/2009/10/9/a-c-challenge.html

by wheels • 17 years ago

33 24 17 years ago

chargen.matasano.com

icefox3• 17 years ago

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

viraptor• 17 years ago

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.

tptacek5• 17 years ago

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.

icefox3• 17 years ago

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.

tptacek5• 17 years ago

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

icefox3• 17 years ago

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

tptacek5• 17 years ago

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.

ice799• 17 years ago

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 :/

vicaya• 17 years ago

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

maxklein• 17 years ago

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.

tptacek5• 17 years ago

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

InclinedPlane• 17 years ago

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.

brown9-22• 17 years ago

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.

mquander• 17 years ago

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.

omouse• 17 years ago

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

fhub• 17 years ago

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)

A C++ Challenge

Related Stories