views:

95

answers:

2

I'm attempting to debug a stack overwrite (no pun intended) corruption problem with some c/c++ code in Visual Studio 2008.

When I compile the solution in win32 debug mode, I'm able to run the debugger and see a class get instantiated.

In the constructor call, we initialize some fixed length char[] member variables using memset.

If I use printf to print the memory location of the member variable, I get a value that is exactly 14 bytes past what the VS2008 watch/local var window reports as the memory location.

How does VS calculate what it thinks the memory address is of a variable?

Edit: I have compiled with Run Time Check options, and what I see is "Run-time Check Failure #2 - stack around variable 'varName' was corrupted.

Edit 3: Here is the abbreviated source

Header file:

//////////////////////////////////////////////////////////////////////

#include "commsock.h"
#include "buffer.h"
#include "BufferedReader.h"

#define AUTHTYPE_PASSKEY    1
#define AUTHTYPE_PAGER      2
#define AUTHTYPE_PASSWORD   3
#define AUTHTYPE_RADIUS     4
#define AUTHTYPE_INFOCARD_RO    5
#define AUTHTYPE_INFOCARD_CR    6

#define AUTHSTATE_NOT_LOGGED_IN 0
#define AUTHSTATE_IDENTIFIED 1
#define AUTHSTATE_AUTHENTICATED 2
#define AUTHSTATE_MAX_FAILS 32
#define AUTHSTATE_NO_LOG 65536

class PRClientSession {
public:
    PRClientSession();
    virtual ~PRClientSession();
    void Reset();

    BOOL BeginAuth(LPCSTR szUserID, LPCSTR szClientIP, LPCSTR szOtherUserID="");
    BOOL CompleteAuth(LPCSTR szResponse);
    BOOL Logoff();
    BOOL DetachAsync();
    BOOL Detach();
    BOOL Attach(const char *authstr=NULL);
    BOOL Connected(){return m_Socket.Connected();};

    BOOL EncryptText(char *pPassword, char *pOut, char *pKey);
    BOOL EncryptText(char *pPassword, char *pOut);
    BOOL DecryptText(char *pPassword, char *pOut, char *pKey);
    BOOL DecryptText(char *pPassword, char *pOut);

    LPCSTR Error(LPCSTR szErr=NULL){if (szErr)strncpy_s(m_szError,szErr,sizeof(m_szError));return m_szError;};
    LPCSTR Challenge(LPCSTR szChal=NULL){if (szChal)strncpy_s(m_szChallenge,szChal,sizeof(m_szChallenge));return m_szChallenge;};
    LPCSTR UserID(LPCSTR szUID=NULL){if (szUID)strncpy_s(m_szUserID,szUID,sizeof(m_szUserID));return m_szUserID;};
    LPCSTR SessionID(LPCSTR szSID=NULL){if (szSID)strncpy_s(m_szSessionID,szSID,sizeof(m_szSessionID));return m_szSessionID;};
    LPCSTR SessionLogID(LPCSTR szSLID=NULL){if (szSLID)strncpy_s(m_szSessionLogID,szSLID,sizeof(m_szSessionLogID));return m_szSessionLogID;};
    LPCSTR SessionStateID(LPCSTR szSSID=NULL){if (szSSID)strncpy_s(m_szSessionStateID,szSSID,sizeof(m_szSessionStateID));return m_szSessionStateID;};
    int AuthType(int iType=-1){if (iType != -1)m_iAuthType = iType;return m_iAuthType;};

    int SendRequest(LPCSTR szType, ...);
    int Write(char *szBuf, int iLen=-1);
    int Read(char *szBuf, int iLen, int iTimeout=-1);
    BOOL ReadResponse(int iBlock=FALSE);
    int ReadResponseTimeout(int iBlock) ;

    BOOL GetField(LPCSTR szField, char *szBuf, int iLen, int total=-1);
    BOOL GetField(LPCSTR szField, int &iBuf );
    char *GetData(){return m_pData;};
    int GetDataSize(){return m_iDataSize;}
    char *GetMessage(){return m_pMessage;};
    SOCKET getSocket(){return m_Socket.getSocket();}

private:
    BOOL LoadConfig();
    BOOL GetMsgField(LPCSTR szIn, LPCSTR szSrch, char *szBuf, int iLen, int total=-1);
    int ReadMessage( char *buf, int len, const char *terminator = NULL );
public:

    bool sendCommand(char* szCommand, char *szArglist);

    LPCSTR ReplyMessage(){ return m_szReplyMessage; }
    int IsRadiusChallenge(){ return m_iIsRadiusChallenge; }

    static char PRIISMS_USER_TAG[];
    static char DEST_USER_TAG[];
    static char RADIUS_USER_TAG[];
    static char RADIUS_PASSWORD_TAG[];
    static char GENERATE_LOGIN[];
    static char VALIDATE_LOGIN[];
    static char PR_RADIUS_GENERATED[];

private:
    BOOL doConnect();

    // Response reader vars...
    char *m_pMessage;
    char *m_pData;
    Buffer m_Buf;
    BOOL m_bLoaded;
    BufferedReader m_reader;

    Buffer m_ReqBuf;

    CCommSocket m_Socket;
    char m_szServer[128];
    int m_iServerPort;

    char m_szError[128];
    int m_iAuthState;
    int m_iDataSize;

    int m_iAuthType;
    char m_szChallenge[1024];
    char m_szUserID[64];
    char m_szSessionID[64];
    char m_szSessionLogID[64];
    char m_szSessionStateID[64];
    char m_szSessionSecret[16];

    long m_RequestID;

    int m_iIsRadiusChallenge;
    char m_szReplyMessage[1024];

};

and the source code with constructor...

#include "stdafx.h"
#include "PRclntsn.h"
#include "iondes.h"
#include "prsystemparameters.h"
#include "prsessionlog.h"
#include "util.h"
#include "PRClntSn.h"

#include <string>
using namespace std;

#define LoadConfigFailedMsg "Unable to retrieve database configuration entries."
#ifndef AUTH_TYPE_RADIUS
#define AUTH_TYPE_RADIUS 4
#endif
//------------------------------------------------------------------------------------

// initialize static members
char PRClientSession::DEST_USER_TAG[] = "DEST_USER=";
char PRClientSession::PRIISMS_USER_TAG[] = "PRIISMS_USER=";
char PRClientSession::RADIUS_USER_TAG[] = "RADIUS_USER=";
char PRClientSession::RADIUS_PASSWORD_TAG[] = "RADIUS_PASSWORD=";
char PRClientSession::GENERATE_LOGIN[] = "GENERATE_LOGIN";
char PRClientSession::VALIDATE_LOGIN[] = "VALIDATE_LOGIN";
char PRClientSession::PR_RADIUS_GENERATED[] = "PR_RADIUS_GENERATED";

PRClientSession::PRClientSession()
{
    Reset();
}



void PRClientSession::Reset()
{
//LOG4CXX_TRACE(mainlogger, CLASS_METHOD_TAG);

    printf("m_szServer mem location: %p", (void *)&m_szServer);
    memset(m_szServer, 0, sizeof(m_szServer));
    m_iServerPort = 0;

    memset(m_szError, 0, sizeof(m_szError));
    m_iAuthState = 0;
    m_iDataSize = 0;

    m_iAuthType = 0;
    memset(m_szChallenge, 0, sizeof(m_szChallenge));
    memset(m_szUserID, 0, sizeof(m_szUserID));
    memset(m_szSessionID, 0, sizeof(m_szSessionID));
    memset(m_szSessionLogID, 0, sizeof(m_szSessionLogID));
    memset(m_szSessionStateID, 0, sizeof(m_szSessionStateID));
    memset(m_szSessionSecret, 0, sizeof(m_szSessionSecret) );
    //    memset(m_szReadBuf, 0, sizeof(m_szReadBuf));

    m_RequestID = 0;
    m_iIsRadiusChallenge = 0;
    memset(m_szReplyMessage, 0, sizeof(m_szReplyMessage));
    m_reader.setComm(&m_Socket);
}

Output of printf:

m_szServer mem location: 01427308

Visual Studio 2008 Locals window

m`_szServer 0x014272fa ""   char [128]`

It's off by 14... and when the code actually runs the very first memset, it does so starting with address 7308, not 72fa. It basically tramples contiguous memory regions (and thus, variables)

+1  A: 

The debugger inserts extra space. This space is set to a pre-defined value, and if it's changed, the debugger knows that something has gone wrong. It's normal for compilers and debuggers to insert space that serves no apparent purpose.

If you have used a fixed-size char[], that immediately signals to me as a problem, since there's no explicit encapsulation. You could use a custom array type (such as boost::array) to write your own buffer overrun detection code. If you throw at this time, you will get a stack trace.

DeadMG
It's a mix of C/C++ code - the project works in Visual Studio 6.0. We've moved the code to VS2008, and now the debugger isn't displaying things correctly.
MattLear
@MattLear: The first thing to do is update your compiler. VS6.0 is notorious, and not in a good way. There's a perfectly fine chance that you just found a bug. Secondly, you've really got to start posting about the C parts of your code, but I'm guessing that you didn't instantiate a class in that section, so my post still applies. Edit: Wait, your OP says that you're in VS9. What compiler are you using, exactly?
DeadMG
@DeadMG - hear hear. VC6 runs stuff 'OK' and then new compiler uncovers simple bugs. I've seen this too. Also for use of boost::array or scoped_array
Steve Townsend
DeadMG: Code was originally developed using VS 6.0 C++. We have migrated project to VS2008 C++ and it compiles with no errors (yes, lots of warnings, and yes, I know they can be a problem). I guess I'm still trying to understand how the VS memory tool can misreport memory locations?
MattLear
DeadMG
@DeadMG and @Steve - I'll try and upload some of the C code to better illustrate what I'm working with. I understand that the debugger may pad space, but if that's the case, I should still be able to see a variable's data change correctly. In this case, for a char[128], I see the data change at element 14, and then carries over to the next contiguous memory block (which holds an int variable and another fixed char array). Years of Java coding has apparently eroded my pointer skillset! I appreciate the responses - really!
MattLear
A: 

This is (probably) not going to fix your bug, but I would try to avoid C-style arrays if you can.

Unless you have some compelling reason to use the fixed-length arrays, replace them with std::vector which will be filled with 0x00 by default.

so instead of

const size_t MYCLASS::BUFLEN(16);

    class myClass {
    public:
      myClass()
      {
        memset(buffer, 0, BUFLEN);
      }
    private:
      static const size_t BUFLEN;
      char buffer[BUFLEN];
    };

you have

const size_t MYCLASS::BUFLEN(16);

    class myClass {
    public:
      myClass() : buffer(BUFLEN)
      {
        memset(buffer, 0, BUFLEN);
      }
    private:
      static const size_t BUFLEN;
      std::vector<char> buffer;
    };
Steve Townsend