Stack around variable corrupted

[Cha0sBG]

Alright so i'm gonna be short and explain as well as i can.

What's my project? : It's a packet analyzer that i made.

What error i get?: Run-Time Check Failure #2 - Stack around the variable 'Buffer' was corrupted

Following code is the functions i use to: Receve the packets, add the data to edit control. Normally the text i want is added with this function but it gives error when the packets are receved.

Receve function:

char PacketBuffer[4096] = " ";
        recv(sConnection, PacketBuffer, sizeof(PacketBuffer), NULL);
             if(PacketBuffer == " ")
                {
                 }else
                     {
                         ApendText(PacketBuffer);
                     }

ApendText function:

char Buffer[4096] = " ";
        GetDlgItemText(uhWnd, LOG, Buffer, 4096);
            strcat(Buffer, "\r\n");
                strcat(Buffer, Text.c_str( ));
                    strcat(Buffer, "\r\n");
                        SetDlgItemText(uhWnd, LOG, Buffer);

Any help would be great.

[Paul McKenzie]

char Buffer[4096] = " ";
GetDlgItemText(uhWnd, LOG, Buffer, 4096);
strcat(Buffer, "\r\n");

So what happens if Buffer is too small when you try to concatenate characters? You are potentially trying to stuff more than 4096 characters into the buffer.

There are much safer ways to do this without assuming your buffer will always be less than 4,096 characters. For one, you can get the length of the text first, by calling GetWindowTextLength(), and then create a buffer of that size using std::vector<char> or using new[]/delete[].

Regards,

Paul McKenzie

[Cha0sBG]

i don't think that buffer is too small for 1-2 packets ecrypted with blowfish.... it's not enoth simbols to overload it..

[Paul McKenzie]

Originally Posted by Cha0sBG

i don't think that buffer is too small for 1-2 packets ecrypted with blowfish.... it's not enoth simbols to overload it..

You don't write programs this way, using the mindset that "I think it's OK, so I need not check anything". You check for all potential situations, no matter how sure you may be.

The very first function call GetDlgItemText can potentially fill your entire buffer. You have a buffer that is 4,096 characters, and then right away, you're calling a function that can fill the buffer, then another (strcat) that adds more characters to this buffer. That is flawed code, regardless of whether you think it will not happen. You don't even check to see if the function failed, and that is one of the potential return values from GetDlgItemText. What if GetDlgItemText returns a failure code? As a matter of fact, you check no return values from any of those functions (recv() is another one).

You should check the return value of GetDlgItemText to ensure that you will not overflow the buffer. You can't write a program, and hope that the function returns correct results -- you have to write it so that if it doesn't return the results you expected, you have a way of dealing with it.

Also, we have no idea where your program is breaking down, since we can't run what you wrote (it isn't a complete program). A memory error could have occurred long before the crash actually happens, so it doesn't help posting the code that finally breaks down -- we need to see the whole application in progress to determine what is causing the problem.

Given this, the only thing we have to work with is what you posted, and certainly it is flawed given the explanation above. There is nothing else in that code that is an obvious corruption.

Regards,

Paul McKenzie

[Cha0sBG]

The problem is i'm not that good with vectors. And the right way was with a Unsigned Char. But seems to crash anyway. No problem with a console app but when i try with the GUI it fails .... The problem is that MS didn't make a function for adding a new line of text to a textbox and i need to try 10000 things before it works. And it seems that strcat() it's very useful for unsigned chars. So now i either need to figure out how to do the vector thing or find a new way to append 1 unsigned char to another.

[hoxsiew]

It only takes a couple of line to append to a multiline edit (I assume that's what you mean by textbox). Send a EM_SETSEL with -1, -1 for WPARAM and LPARAM then send a EM_REPLACESEL with the buffer to add as the LPARAM. See MSDN for more info.

[Cha0sBG]

Actually , now i use only the following:

SendDlgItemMessage(hWnd, LOG, EM_REPLACESEL, 0, "The Text\r\n");

And works fine xP hope that now i have more options ..