5 ms·

A C++ Challenge

33 points by wheels 17y ago

vicaya 17y ago
I'd argue that the integer overflow inside new implementation is a compiler bug.
icefox 17y 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 17y 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.
  - icefox 17y ago
    On the topic of NULL the Stroustrup answer: http://www.research.att.com/~bs/bs_faq2.html#null http://www.research.att.com/~bs/bs_faq2.html#null Really it is just another hint that this is made by a C programmer.
  - tptacek 17y 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.
    - 0wned 17y ago
      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.
      - copper 17y ago
        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?
- tptacek 17y ago
  This code has a vulnerability that is only present because it is C++, not C. Find it, and then talk about the hygiene.
  - icefox 17y 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
    - tptacek 17y ago
      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.
      - jrockway 17y ago
        I'm starting to see why I don't use intrinsically unsafe languages like C and C++.
      - huhtenberg 17y ago
        > 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.
    - brown9-2 17y ago
      I think you are really missing the point of this exercise...
tptacek 17y 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 17y 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 :/
maxklein 17y 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.
- tptacek 17y ago
  You can't isolate this problem, because it only occurs when you have an adversary.
  - InclinedPlane 17y 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.
    - pwmanagerdied 17y ago
      Wonderfully entertaining analogy.
- brown9-2 17y 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.
omouse 17y ago
The problem is you're using C/C++ and dealing with pointers directly. What do I win?
fhub 17y 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)

33 points by wheels 17y ago

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

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 17y 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.
- icefox 17y ago
  On the topic of NULL the Stroustrup answer: http://www.research.att.com/~bs/bs_faq2.html#null http://www.research.att.com/~bs/bs_faq2.html#null Really it is just another hint that this is made by a C programmer.
- tptacek 17y 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.
  - 0wned 17y ago
    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.
    - copper 17y ago
      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?
tptacek 17y ago
This code has a vulnerability that is only present because it is C++, not C. Find it, and then talk about the hygiene.
- icefox 17y 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
  - tptacek 17y ago
    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.
    - jrockway 17y ago
      I'm starting to see why I don't use intrinsically unsafe languages like C and C++.
    - huhtenberg 17y ago
      > 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.
  - brown9-2 17y ago
    I think you are really missing the point of this exercise...

tptacek 17y 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 17y 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 :/

maxklein 17y 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.

tptacek 17y ago
You can't isolate this problem, because it only occurs when you have an adversary.
- InclinedPlane 17y 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.
  - pwmanagerdied 17y ago
    Wonderfully entertaining analogy.
brown9-2 17y 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.

omouse 17y ago

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

fhub 17y 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)