Win32 Buffer Overflow

[newguy194]

I am writing a GUI for a program which scrambles text messages, but I believe I am running into buffer overflows somewhere and have tried various ways of stopping the overflows from happening. When the user input text, it encrypts fine, but when the user decrypts text, they will get various strings appended to the end from memory, such as windir=C:\Windows, and other random strings.

#define IDB_BUTTON 101
#define IDB_EDIT 102
#define IDB_EDITDATA 103
#define BTE_DECRYPT 104
#include <windows.h>
#include <string>
 
 
 
using namespace std;
string toencrypt;
string key;
int decryptencrypt = 1;
char* encrypted;
char* decrypted;
string todecrypt;
int length;
    HWND       decryptbutton;           
    HWND       encryptedfield;
    HWND       databoxLabel;
    HWND       datafield;
    HWND       keyLabel;
    HWND       KEY;
    HWND       BUTTON;
    MSG        Msg;
    HWND       hWnd;
    HRESULT       hRet;
    WNDCLASSEX WndClsEx;
 
void encrypt()
{
     if(toencrypt=="" || toencrypt==" ")
     {}
     else
     {
     int maxlen = key.length();
    string encrypte = "%";
          decryptencrypt = 0;
                   for(int x = 0;x<key.length();x++)
                   {
                           if(key[x] == '1')
                           {
                               toencrypt.assign(toencrypt.rbegin(), toencrypt.rend());
                           }
                           else if(key[x] == '2')
                           {
                              for(int x = 0;x<toencrypt.length();x++)
                              {
                                 if(toencrypt[x]==' ')
                                 {
                                   toencrypt.replace(x,1,encrypte);
                                   }
                                   else{}
                              }
                           }
                           else if(key[x] == '3')
                           {
 
                           }
                           else if(key[x] == '4')
                           {
 
                           }
                           else if(key[x] == '5')
                           {
                           }
                           else if(key[x] == '6')
                           {
                           }
                           else if(key[x] == '7')
                           {
                           }
                           else if(key[x] == '8')
                           {
                           }
                           else if(key[x] == '9')
                           {
                           }
                           else if(key[x] == '0')
                           {
                           }
                           else{MessageBox(NULL, "INVALID KEY", "ERROR", MB_OK);}
                           }
                   encrypted = new char[toencrypt.length() + 10];
                   for(int x = 0;x<toencrypt.length() + 1;x++)
                   {
                      encrypted[x] = toencrypt[x];
                   }
              SetDlgItemText(hWnd, IDB_EDITDATA, encrypted);
              toencrypt = "";
              }
 
 
}
void decrypt()
{
    if(todecrypt=="" || todecrypt==" ")
    {}
    else
    {
  string encrypte2 = " ";
         decryptencrypt = 1;
     key.assign(key.rbegin(), key.rend());
     for(int x = 0;x<key.length();x++)
                   {
                           if(key[x] == '1')
                           {
                               todecrypt.assign(todecrypt.rbegin(), todecrypt.rend());
                           }
                           else if(key[x] == '2')
                           {
                                for(int x = 0;x<todecrypt.length();x++)
                              {
                                 if(todecrypt[x]=='%')
                                 {
                                   todecrypt.replace(x,1,encrypte2);
                                   }
                                   else{}
                                   }
                           }
                           else if(key[x] == '3')
                           {
 
                           }
                           else if(key[x] == '4')
                           {
 
 
                           }
                           else if(key[x] == '5')
                           {
                           }
                           else if(key[x] == '6')
                           {
                           }
                           else if(key[x] == '7')
                           {
                           }
                           else if(key[x] == '8')
                           {
                           }
                           else if(key[x] == '9')
                           {
                           }
                           else if(key[x] == '0')
                           {
                           }
                           else{MessageBox(NULL, "INVALID KEY", "ERROR", MB_OK);}
                   }
                   decrypted = new char[todecrypt.length() + 10];
                   for(int x = 0;x<todecrypt.length();x++)
                   {
                     decrypted[x] = todecrypt[x];
                     }
                   SetDlgItemText(hWnd, IDB_EDITDATA, decrypted);
                   todecrypt = "";
                   }
 
}
LRESULT CALLBACK WndProcedure(HWND hWnd, UINT uMsg,
               WPARAM wParam, LPARAM lParam);
 
INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
               LPSTR lpCmdLine, int nCmdShow)
{
 
 
    // Populate the WNDCLASSEX structure
    WndClsEx.cbSize        = sizeof(WNDCLASSEX);
    WndClsEx.style         = CS_HREDRAW | CS_VREDRAW;
    WndClsEx.lpfnWndProc   = WndProcedure;
    WndClsEx.cbClsExtra    = 0;
    WndClsEx.cbWndExtra    = 0;
    WndClsEx.hIcon         = LoadIcon(NULL, IDI_APPLICATION);
    WndClsEx.hCursor       = LoadCursor(NULL, IDC_ARROW);
    WndClsEx.hbrBackground = (HBRUSH)(COLOR_WINDOW);
    WndClsEx.lpszMenuName  = NULL;
    WndClsEx.lpszClassName = "ENCRYPTER";
    WndClsEx.hInstance     = hInstance;
    WndClsEx.hIconSm       = LoadIcon(NULL, IDI_APPLICATION);
 
    // Register the class
    RegisterClassEx(&WndClsEx);
 
    // Create the window object
    hWnd = CreateWindow("ENCRYPTER",
              "ENCRYPTER",
              WS_OVERLAPPEDWINDOW,
              0,
              0,
              800,
              400,
              NULL,
              NULL,
              hInstance,
              NULL);
    decryptbutton = CreateWindow(
    "BUTTON",
    "Decrypt",
    WS_VISIBLE | WS_CHILD | BS_DEFPUSHBUTTON,
    500,
    300,
    150,
    50,
    hWnd,
    (HMENU)BTE_DECRYPT,
    hInstance,
    NULL
    );
    databoxLabel = CreateWindow(
    "STATIC",
    "DATA",
    WS_VISIBLE | WS_CHILD,
    18,
    35,
    100,
    100,
    hWnd,
    NULL,
    hInstance,
    NULL
    );
    datafield = CreateWindow(
    "EDIT",
    "",
    WS_VISIBLE | WS_CHILD | WS_BORDER | ES_LEFT | ES_MULTILINE | WS_VSCROLL,
    60,
    35,
    700,
    250,
    hWnd,
    (HMENU)IDB_EDITDATA,
    hInstance,
    NULL
    );
    keyLabel = CreateWindow(
    "STATIC",
    "KEY",
    WS_VISIBLE | WS_CHILD,
    18, 
    10, 
    100, 
    20,
    hWnd,
    NULL, 
    hInstance,
    NULL
    );
 
    KEY = CreateWindow(
    "EDIT",
    "",
    WS_VISIBLE | WS_CHILD | ES_LEFT | WS_BORDER | ES_NUMBER,
    60, 
    10, 
    700, 
    20,
    hWnd,
    (HMENU)IDB_EDIT,
    hInstance,
    NULL
    );
 
    BUTTON = CreateWindow( 
    "BUTTON",                                   
    "Encrypt",                                        
    WS_VISIBLE | WS_CHILD | BS_DEFPUSHBUTTON,                                   
    300,                                 
    300,                   
    150,                                      
    50,                   
    hWnd,                                     
    (HMENU)IDB_BUTTON,                              
    hInstance,                                   
    NULL                                       
);
 
    if ( !BUTTON)
       return 0;
    // Verify window creation
    if( !hWnd ) // If the window was not created,
        return 0; // stop the application
 
    // Show the window
    ShowWindow(hWnd, SW_SHOWNORMAL);
 
    UpdateWindow(hWnd);
    // our message pump
    while( (hRet = GetMessage( &Msg, NULL, 0, 0 )) != 0)
    { 
        if (hRet == -1)
        {
        // handle the error and possibly exit
        }
        else
        {
            TranslateMessage(&Msg); 
            DispatchMessage(&Msg); 
        }
    }
}
 
//////////////////
// WndProcedure //
//////////////////
 
LRESULT CALLBACK WndProcedure(HWND hWnd, UINT Msg,
               WPARAM wParam, LPARAM lParam)
{
    switch(Msg)
    {
    case WM_COMMAND:
         switch(LOWORD(wParam))
         {
            case IDB_BUTTON:
                length = GetWindowTextLength(GetDlgItem(hWnd, IDB_EDITDATA)) + 1;
                char temp[length];
                GetDlgItemText(hWnd, IDB_EDITDATA, temp, length);
                toencrypt.assign(temp);
                delete [] temp;
                length = GetWindowTextLength(GetDlgItem(hWnd, IDB_EDIT)) + 1;
                char temp2[length];
                GetDlgItemText(hWnd, IDB_EDIT, temp2, length);
                key.assign(temp2);
                delete [] temp2;
                encrypt();
                break;
            case BTE_DECRYPT:
                 length = GetWindowTextLength(GetDlgItem(hWnd, IDB_EDITDATA)) + 1;
                 char temp3[length + 1];
                 GetDlgItemText(hWnd, IDB_EDITDATA, temp3, length);
                 todecrypt.assign(temp3);
                 delete [] temp3;
                 length = GetWindowTextLength(GetDlgItem(hWnd, IDB_EDIT)) + 1;
                 char temp4[length + 1];
                 GetDlgItemText(hWnd, IDB_EDIT, temp4, length);
                 key.assign(temp4);
                 delete [] temp4;
                 decrypt();
                 break;
            default:
                    break;
         }
   break;
    case WM_DESTROY:
        // user wants to exit
        PostQuitMessage(WM_QUIT);
        break;
    default:
        // Hand off unprocessed messages to DefWindowProc
        return DefWindowProc(hWnd, Msg, wParam, lParam);
    }
 
    return 0;
}

[weaknessforcats]

Part of it is here:

for(int x = 0;x<toencrypt.length() + 1;x++)
{
encrypted[x] = toencrypt[x];
}

You are running off the end of the string. The loop should be:

size_t length =  toencrypt.size()
for(int x = 0;x<lkength;x++)
                   {
                      encrypted[x] = toencrypt[x];
                   }
 

The method string::length() is deprecated. You should be using string::size().

Writing loops using STL containers is safer if you use iterators.

Then there's this:
[quote=newguy194]
char temp2[length];
GetDlgItemText(hWnd, IDB_EDIT, temp2, length);
key.assign(temp2);
delete [] temp2;
[/code]

Here you are deleting buffer temp2 that you never allocated. This is a guaranteed run-time crash. Ditto for temp3.

Other observations are:
1) Why all the global variables???
2) What the the 10 for?? Code should not have magic numbers:

decrypted = new char[todecrypt.length() + 10];

3) Lotta string literals that should be in a resource string table. I would have expected an IDS_etc...
4)These belong in a resource table

#define IDB_BUTTON 101
#define IDB_EDIT 102
#define IDB_EDITDATA 103

5) This should be a const unsigned int:

#define BTE_DECRYPT 104

[newguy194]

Thanks for the help, I'm 'trying' to teach myself win32 from online tutorials and some old books belonging to my brother, the books are dated from 1999 through 2002, so I might need a newer one.

[Studlyami]

If your using Win32 those books are fine. I can't really think anything that would change from 2000 on. There was a .dll update that allowed ownerdraw for a few extra controls. Win32 is old and is well on its way out for new development.