Question

Satyaprakash A on Mon, 17 Sep 2018 09:05:41


I am getting the below error when I run my project:

Run-Time Check Failure #2 - Stack around the variable "regObj" was corrupted.

At the end of the below function i am getting this error:

void CPowerChuteApp::InitializeTracking()
{
DWORD adc_status = 0;
const int cADCEnabled = 1;
TrackingData td;

//is application data collectiom enabled?
RegistryObj regObj(APC_DALI_SETTINGS_REG_PATH);
regObj.Get(APC_SETTINGS_ADC_ENABLE, adc_status);

//if application data collection is enabled
if (adc_status = cADCEnabled)
{
//popultate the tracking structure

const int   cRootSize = 4; //size of the root variable
TCHAR  install_dir[MAX_PATH]; //the Install Directory
DWORD  sizeOfBuffer = MAX_PATH;

RegistryObj reg (APC_DALI_DEFAULT_REG_PATH);

//get the install directory path
reg.Get(APC_INSTALL_PATH, install_dir, sizeOfBuffer);

td.bEnableTrackingOnStart = TRUE; //tracking is enabled
td.btAppIdCode = APC_PRODUCT_FAMILY_VERSION; //represent PCPE
td.btFileIdVer = APC_FILE_ID_VERSION;
td.btMajorVersion = APC_MAJOR_VERSION;
td.btMinorVersion = APC_MINOR_VERSION;

_tcscpy(td.szAppId, APC_APP_ID);
_tcscpy(td.szDacExe, install_dir);
_tcscpy(td.szFamily, APC_PRODUCT_VERSION);
_tcscpy(td.szFileIdStr, APC_FILE_ID_STR );
_tcscpy(td.szRegPath, APC_DALI_SETTINGS_REG_PATH);

_tcscat(install_dir, TRACKING_FILE); 
_tcscpy(td.szTrackingFile, install_dir);

m_trackData.InitTracking(td);

        m_trackData.TrackStartedEvn();

        m_trackData.GuiOpenEvn();
}

}

Below are my findings:

In the above function at the below line

TCHAR  install_dir[MAX_PATH];

install_dir is showing the below junk value

ÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌ

Because of that in the below lines also i am getting the same junk values:

_tcscpy(td.szDacExe, install_dir);
_tcscat(install_dir, TRACKING_FILE); 

MAX_PATH  defined in minwindef.h as shown below:

#define MAX_PATH          260

Below is my Get function and it is not assigning any value to "install_dir" And also in this function "theDisposition" is "0" and the control is not going into the if condition.

BOOL RegistryObj::Get (LPCTSTR aValueName, LPTSTR aValue, ULONG &aBufSize)
{
    BOOL result = FALSE;
    DWORD tmp_type =  0;

    if (theDisposition > 0)  {
        if (Get (aValueName, aValue, tmp_type, aBufSize)) {
            assert (tmp_type == REG_SZ);        
            result = TRUE;       
        }
    }

    return result;
}

I am suspecting that it might be due to the MAX_PATH that is provided is not sufficient or no value is assigned to install_dir.

Please suggest me the exact root cause.


        

Replies

RLWA32 on Mon, 17 Sep 2018 12:15:13


First, realize that in a debug build VC++ fills uninitialized variables with a sentinel value.  This is the reason that you see "junk" in the uninitialized install_dir variable. 

Presumably, the install_dir variable is intended to be initialized by -

reg.Get(APC_INSTALL_PATH, install_dir, sizeOfBuffer);

So you should debug your code to ensure that call is performing as intended.

If it is not, and install_dir is filled  with the sentinel values inserted by the debugger then subsequent use of install_dir in -

_tcscpy(td.szDacExe, install_dir);
_tcscat(install_dir, TRACKING_FILE);

are probably causing buffer overruns.

The bottom line is that you need to step through the code in the debugger to determine the point of failure and the underlying cause.


WayneAKing on Mon, 17 Sep 2018 16:46:43




In the above function at the below line

TCHAR  install_dir[MAX_PATH];

install_dir is showing the below junk value

ÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌÌ

Because of that in the below lines also i am getting the same junk values:

_tcscpy(td.szDacExe, install_dir);
_tcscat(install_dir, TRACKING_FILE); 

Initialize the buffer and the debug build won't assign those tracking
"magic values" to it. 

TCHAR  install_dir[MAX_PATH] = {0};

That should prevent overruns from occurring when you use install_dir as
the source for a strcpy when no values have been assigned to it.

You will still have to debug your program to find out why and where it is
failing to assign a value to install_dir as you expected.

- Wayne

Satyaprakash A on Tue, 18 Sep 2018 08:54:03


Hi,

Thank you for the valuable inputs.

When i was debugging the code i found the below information.

Before calling the Get(...) function from the code

reg.Get(APC_INSTALL_PATH, install_dir, sizeOfBuffer);

From the below code we are calling the RegistryObj constructor.

RegistryObj reg (APC_DALI_DEFAULT_REG_PATH);

But in the constructor, "aHKey" argument is showing as "unused" and "aCreateFlag" is "0". And the control is going to the else part but from the if(RegOpenKeyEx(.. the control is coming out.

So, "theDisposition" variable is still showing as "0". I want to know why the control is not going within the loop.

Below is the constructor:

RegistryObj::RegistryObj(LPCTSTR aSubKeyName, HKEY aHKey, BOOL aCreateFlag)
:theHKey(NULL),
 theDisposition(0),
 theSubKeyIndex(0),
 theBaseKey(aHKey)
{
    if(aSubKeyName != NULL) {
        assert(_tcslen(aSubKeyName) < (MAX_PATH - 1));
        _tcscpy(theBaseRegPath, aSubKeyName);

        if(aCreateFlag) {
            if(RegCreateKeyEx(aHKey, aSubKeyName, 0,
                NULL, REG_OPTION_NON_VOLATILE, 
                KEY_ALL_ACCESS, NULL, &theHKey, 
                &theDisposition) != ERROR_SUCCESS) {
                assert("RegistryObj::RegistryObj()" == NULL);
            }
        } 
        else {
            if(RegOpenKeyEx(aHKey, theBaseRegPath, 0, KEY_READ, &theHKey) == ERROR_SUCCESS) {
                theDisposition = REG_OPENED_EXISTING_KEY;
            } 
        }
    }
}

And also when i am trying to see the definition of the RegOpenKeyEx function, it is going to Winreg.h using the below path

C:\Program Files\Windows Kits\10\Include\10.0.17134.0\um\Winreg.h

#ifdef UNICODE

#define RegOpenKeyEx RegOpenKeyExW

#else

#define RegOpenKeyEx RegOpenKeyExA

#endif

Please provide your thoughts on this.

RLWA32 on Tue, 18 Sep 2018 11:19:56


Hi,

Thank you for the valuable inputs.

When i was debugging the code i found the below information.

Before calling the Get(...) function from the code

reg.Get(APC_INSTALL_PATH, install_dir, sizeOfBuffer);

From the below code we are calling the RegistryObj constructor.

RegistryObj reg (APC_DALI_DEFAULT_REG_PATH);

But in the constructor, "aHKey" argument is showing as "unused" and "aCreateFlag" is "0". And the control is going to the else part but from the if(RegOpenKeyEx(.. the control is coming out.

So, "theDisposition" variable is still showing as "0". I want to know why the control is not going within the loop.

Below is the constructor:

RegistryObj::RegistryObj(LPCTSTR aSubKeyName, HKEY aHKey, BOOL aCreateFlag)
:theHKey(NULL),
 theDisposition(0),
 theSubKeyIndex(0),
 theBaseKey(aHKey)
{
    if(aSubKeyName != NULL) {
        assert(_tcslen(aSubKeyName) < (MAX_PATH - 1));
        _tcscpy(theBaseRegPath, aSubKeyName);

        if(aCreateFlag) {
            if(RegCreateKeyEx(aHKey, aSubKeyName, 0,
                NULL, REG_OPTION_NON_VOLATILE, 
                KEY_ALL_ACCESS, NULL, &theHKey, 
                &theDisposition) != ERROR_SUCCESS) {
                assert("RegistryObj::RegistryObj()" == NULL);
            }
        } 
        else {
            if(RegOpenKeyEx(aHKey, theBaseRegPath, 0, KEY_READ, &theHKey) == ERROR_SUCCESS) {
                theDisposition = REG_OPENED_EXISTING_KEY;
            } 
        }
    }
}

And also when i am trying to see the definition of the RegOpenKeyEx function, it is going to Winreg.h using the below path

C:\Program Files\Windows Kits\10\Include\10.0.17134.0\um\Winreg.h

#ifdef UNICODE

#define RegOpenKeyEx RegOpenKeyExW

#else

#define RegOpenKeyEx RegOpenKeyExA

#endif

Please provide your thoughts on this.

RegOpenKeyExA/RegOpenkeyExW are standard registry functions provided as a part of Windows API. 

Are you the author of the RegistryObjj class or did you obtain the code from some other source?

WayneAKing on Tue, 18 Sep 2018 15:19:25



And also when i am trying to see the definition of the RegOpenKeyEx function, it is going to Winreg.h using the below path

C:\Program Files\Windows Kits\10\Include\10.0.17134.0\um\Winreg.h

#ifdef UNICODE

#define RegOpenKeyEx RegOpenKeyExW

#else

#define RegOpenKeyEx RegOpenKeyExA

#endif

Please provide your thoughts on this.

No surprises there. Many of the Win API functions map to different calls
depending on whether or not it is a UNICODE build. If it is, then a 
wide-string version of the function is used. This is denoted by a W at the 
end of the function name. Otherwise the function name with an A appended
is used with narrow-strings.

RegOpenKeyExW function
https://docs.microsoft.com/en-us/windows/desktop/api/winreg/nf-winreg-regopenkeyexw

RegOpenKeyExA function
https://docs.microsoft.com/en-us/windows/desktop/api/winreg/nf-winreg-regopenkeyexa

- Wayne

Satyaprakash A on Tue, 18 Sep 2018 16:30:37


RegistryObj class is our own class and it is like as shown below:

class RegistryObj {

public:
    RegistryObj          (LPCTSTR aBaseRegPath = NULL, HKEY aHKey = HKEY_LOCAL_MACHINE, BOOL aCreateFlag = FALSE);
    ~RegistryObj         ();

    BOOL Get             (LPCTSTR aValueName, LPTSTR aValue, ULONG &aBufSize);
    BOOL Get             (LPCTSTR aValueName, LPTSTR aValue, ULONG *pBufSize);
    BOOL Get             (LPCTSTR aValueName, LPTSTR aValue);
    BOOL Get             (LPCTSTR aValueName, ULONG &aValue);
    BOOL GetNum          (LPCTSTR aValueName, ULONG *pValue);
    BOOL Get             (LPCTSTR aValueName, LPTSTR &aValue, DWORD &aType, DWORD &aBufSize);
    BOOL Set             (LPCTSTR aValueName, ULONG aValue);    
    BOOL Set             (LPCTSTR aValueName, LPTSTR aValue, ULONG aBufSize);

    //BOOL GetNextSubKey   (LPTSTR  aValue, ULONG &aBufSize);
    //BOOL GetNextSubKey   (LPTSTR  aValue, ULONG &aBufSize,
    //                      LPTSTR  aClassValue, ULONG &aClassBufSize);
    BOOL AlreadyExisted  ();
    inline void ResetSubKeyEnum () { theSubKeyIndex = 0; }

private:
    TCHAR theBaseRegPath [MAX_PATH];
HKEY  theBaseKey;
    HKEY  theHKey;
    DWORD theDisposition;
    DWORD theSubKeyIndex;
};

RLWA32 on Tue, 18 Sep 2018 16:40:37


RegistryObj class is our own class and it is like as shown below:

class RegistryObj {

public:
    RegistryObj          (LPCTSTR aBaseRegPath = NULL, HKEY aHKey = HKEY_LOCAL_MACHINE, BOOL aCreateFlag = FALSE);
    ~RegistryObj         ();

    BOOL Get             (LPCTSTR aValueName, LPTSTR aValue, ULONG &aBufSize);
    BOOL Get             (LPCTSTR aValueName, LPTSTR aValue, ULONG *pBufSize);
    BOOL Get             (LPCTSTR aValueName, LPTSTR aValue);
    BOOL Get             (LPCTSTR aValueName, ULONG &aValue);
    BOOL GetNum          (LPCTSTR aValueName, ULONG *pValue);
    BOOL Get             (LPCTSTR aValueName, LPTSTR &aValue, DWORD &aType, DWORD &aBufSize);
    BOOL Set             (LPCTSTR aValueName, ULONG aValue);    
    BOOL Set             (LPCTSTR aValueName, LPTSTR aValue, ULONG aBufSize);

    //BOOL GetNextSubKey   (LPTSTR  aValue, ULONG &aBufSize);
    //BOOL GetNextSubKey   (LPTSTR  aValue, ULONG &aBufSize,
    //                      LPTSTR  aClassValue, ULONG &aClassBufSize);
    BOOL AlreadyExisted  ();
    inline void ResetSubKeyEnum () { theSubKeyIndex = 0; }

private:
    TCHAR theBaseRegPath [MAX_PATH];
HKEY  theBaseKey;
    HKEY  theHKey;
    DWORD theDisposition;
    DWORD theSubKeyIndex;
};

It seems you need to further debug your own code.

Posting minimal code snippets in the forum doesn't give us enough information to diagnose the issues or reproduce them on our own systems.

I suggest you produce a small demo that reliably reproduces the problem and make it available to the community by sharing it on OneDrive.

WayneAKing on Tue, 18 Sep 2018 20:44:45


Would I be correct if I assumed that you are working on software for the
APC UPS Network Management Card and PowerChute Network Shutdown?

- Wayne