3

I am new to C++.

Can any C++ expert tell me would this causing buffer overflow?

Sample Code:

MyObject op;
memset(&op, 0, sizeof(MyObject));

On my view it is ok to be this since the limit is the size of the object type itself but there is a security tools caught this as a buffer overflow. If it is really a buffer overflow, can you please describe a bit how could it be. Thank you for your clarification and explanation.

  • 1
    Which security tools are you using? – Mark Sep 25 '14 at 9:27
  • 2
    Come now, who voted this as off-topic? It's about the security of a particular operation in code. If that's not security, I don't know what is. Get some itch cream for your trigger finger. – Polynomial Sep 25 '14 at 9:35
  • It's about programming, and as the quality of answers so far shows, it would be much better handled over on StackOverflow. – Mark Sep 25 '14 at 9:38
  • @Mark It's absolutely on topic here. Whether someone could answer it better elsewhere is irrelevant. In these situations you can advise OP to ask over on another site, because they'd have better expertise, but that's not a valid reason alone to flag something to be closed as off-topic. – Polynomial Sep 25 '14 at 9:43
  • I also wonder if it is a false positive, but I cannot confirm it, cause I not familiar with c++. I am not sure the behavior of sizeof() and memset() in c++, I wonder whether is there anything will affect the size of the MyObject because I read this (stackoverflow.com/questions/119123/…). Will the variables of the Object has discontinuous memory address? – overshadow Sep 25 '14 at 15:50
2

This is not a buffer overflow. The buffer size you are giving to memset is exactly the actual size of the buffer (in this case, the object op).

If this is a C++ object with virtual functions, then the memset will zero memory that should not have been zeroed, and the call to the destructor is likely to blow up. But this would be memory corruption, not a buffer overflow.


As for the reason that your tool complains: there is a common idiom that uses the type of memset's first argument to determine the buffer size:

memset(&my_object, 0, sizeof(my_object));
memset(my_pointer, 0, sizeof(*my_pointer));
memset(my_array, 0, sizeof(my_array));

It's possible that your tool throws a warning whenever you use anything else for the size, because you might forget to update the sizeof when you later change the actual object.

  • It could be that the tool in use considers clobbering the virtual function table to be an overflow, but without knowing which tool is involved, it's impossible to say. – Mark Sep 25 '14 at 20:04
1

Ignore me, I'm an idiot.

Mark points out that MyObject op is a local-scope instantiation of a class, which will be placed on the stack. This means that &op is actually the address of the class data in memory.

As such, I have no idea why this was flagged. False positive, maybe?


It's right to complain, so let's take a look at this code. What does it do?

MyObject op;

In line 1, you define an object variable. However, you don't call its constructor, so this object variable isn't actually set to anything - right now it's just an uninitialised pointer.

memset(&op, 0, sizeof(MyObject));

In line 2, you take the address of the variable and and perform a memset at that address with a size that's almost certainly going to be larger than the size of a pointer. Take care to note that you aren't overwriting the memory that will contain the object's data, but rather the scope-local storage space that's used to hold a pointer to the object. This is definitely an overflow condition.

  • 1
    Wrong on all counts. The first line creates an object, allocating a suitable amount of memory for it (most likely on the stack), and calls its default constructor. The second line takes the address of that object, and fills the memory allocated for the object with zeroes. – Mark Sep 25 '14 at 9:30
  • @Mark Admittedly I've not used C++ for a while (I'm more used to C), but my understanding was that MyObject op; alone didn't call the default constructor, whereas MyObject op(); does. Also, surely &op doesn't refer to the object itself (since op is a pointer type) but rather the address of the pointer? – Polynomial Sep 25 '14 at 9:33
  • 1
    MyObject op; creates an object of type MyObject. MyObject *op; creates a pointer to an object of type MyObject. – Mark Sep 25 '14 at 9:36
  • @Mark Ah, yes, so it does - it's a local scope instance. Blame my lack of coffee this morning! Rather odd that this is flagged, then. – Polynomial Sep 25 '14 at 9:38
0

It depends on the definition of MyObject. Since it's C++, the constructor has set up the initial values which you then overwrite using memset. The destructor will assume you have not overwritten the object, and could exhibit all kinds of unintended behavior.

But since you don't show any of that code, we have to assume the worst. Memory overflows are quite possible.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.