CODEWITHSUNDEEP
winapiregistryinstallshield
msiyer
msiyer

Reputation: 841

Invalid/Foreign characters in value fetched from registry

I have an installer created using Installshield 2012 that depends greatly on the value of a Registry key written by some other application developed in-house. I use RegDBGetKeyValueEx Installscript API to fetch the value.

This value is a directory path guaranteed to have a trailing backslash. This value serves as the TARGETDIR for my installer.

The issue is that I often see a foreign character (Chinese, Japanese or Korean) appended to the backslash.This causes my TARGETDIR to contain the foreign character. This pollutes my installation.

I feel this is some bug with Installscript API which wrongly translates the Registry value.

Please provide some inputs so that I may find the root-cause of the issue.

EDIT 1:

The other application in question is a purely ANSI application with no support for Unicode. There is no #define UNICODE specified anywhere. I also checked the compiler parameters and found nothing related to Unicode specified.

This application reads one Registry key and appends a backslash and updates other keys. Once this operation is done my installer fetches the values written:

if ((RegOpenKeyEx(HKEY_LOCAL_MACHINE, csVersKey, 0, KEY_READ|KEY_WRITE, &hKey) == ERROR_SUCCESS) &&
        (RegQueryValueEx(hKey, "BasePath", NULL, NULL, (unsigned char *)szTemp, &BufferSize) == ERROR_SUCCESS))
    {
        m_reqPath = szTemp;
        if (m_regPath.Right(1) != "\\") m_reqPath += "\\";
        m_reqMachinePath = m_reqPath + "\\" + m_reqMachine + "\\";
        m_reqHardwarePath = m_reqMachinePath + HARDWARE_DIRECTORY;
        m_reqManualPath = m_reqMachinePath + MANUAL_DIRECTORY;
        m_reqVersionPath = m_reqMachinePath + m_reqVersion + "\\";
        m_reqFirmwarePath = m_reqVersionPath + FIRMWARE_DIRECTORY;
        m_RegVersDirectory = m_reqVersionPath + PROGRAM_DIRECTORY;
        RegCloseKey(hKey);
    }

The values updated in the above code are then written to registry. The Installscript code that fetches these values is:

if ((RegDBKeyExist(szBaseKey ^ szRegKey) = 1) && (RegDBGetKeyValueEx(szBaseKey ^ szRegKey, "ReqPath", nTemp, szTemp, nvSize) = 0)) then

EDIT 2:

The way the registry value is set seems to be wrong:

RegSetValueEx(hKey, LEICA_PATH, 0, REG_SZ, (const unsigned char*)m_reqLeicaPath.GetBuffer(m_reqLeicaPath.GetLength()), m_reqLeicaPath.GetLength());

I feel the correct code should have been:

RegSetValueEx(hKey, LEICA_PATH, 0, REG_SZ, (const unsigned char*)m_reqLeicaPath.GetBuffer(m_reqLeicaPath.GetLength()), m_reqLeicaPath.GetLength() + 1);

Upvotes: 0

Views: 670

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 597036

When assigning the Registry data to your m_reqPath variable, you are treating it as null-terminated character data. However, string data read by RegQueryValueEx() is not guaranteed to be null-terminated, if the original writer of the data did not include one. This is clearly stated in the documentation.

Chances are that the string data you are reading from the Registry is not null-terminated, so you end up copying random data from the end of your buffer. To account for that possibility, you need to allocate extra space in your buffer and put your own null terminator at the end of whatever data you actually read. If the data is properly null terminated, then your terminator will simply be redundant.

You are also leaking the opened Registry key handle if RegOpenKeyEx() succeeds but RegQueryValueEx() fails.

Try this instead:

if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, csVersKey, 0, KEY_QUERY_VALUE, &hKey) == ERROR_SUCCESS)
{
    char szTemp[257];
    DWORD BufferSize = 256;

    if (RegQueryValueEx(hKey, "BasePath", NULL, NULL, (BYTE*)szTemp, &BufferSize) == ERROR_SUCCESS)
    {
        szTemp[BufferSize] = '\0';
        m_reqPath = szTemp;
        ...
    }

    RegCloseKey(hKey);
}

To avoid this issue altogether, use RegGetValue() instead, which deals with the null terminator for you:

char szTemp[257];
DWORD BufferSize = sizeof(szTemp);

if (RegGetValue(HKEY_LOCAL_MACHINE, csVersKey, "BasePath", RRF_RT_REG_SZ | RRF_RT_REG_MULTI_SZ | RRF_RT_REG_EXPAND_SZ, NULL, szTemp, &BufferSize) == ERROR_SUCCESS)
{
    m_reqPath = szTemp;
    ...
}

Or:

if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, csVersKey, 0, KEY_QUERY_VALUE, &hKey) == ERROR_SUCCESS)
{
    char szTemp[257];
    DWORD BufferSize = sizeof(szTemp);

    if (RegGetValue(hKey, NULL, "BasePath", RRF_RT_REG_SZ | RRF_RT_REG_MULTI_SZ | RRF_RT_REG_EXPAND_SZ, NULL, szTemp, &BufferSize) == ERROR_SUCCESS)
    {
        m_reqPath = szTemp;
        ...
    }

    RegCloseKey(hKey);
}

Upvotes: 1

Jonathan Potter
Jonathan Potter

Reputation: 37192

In some code pages the currency symbol is the same byte value in ASCII as the backslash. If you're using Unicode functions this shouldn't be an issue though.

See http://www.siao2.com/2005/09/17/469941.aspx for more information.

Upvotes: 0

Related Questions