Error with function call CRegKey::QueryStringValue

Question

Today i found this strange error in querying the time zone information . The code to read the display name of time zones looks like this

typedef struct {
    LONG Bias;
    LONG StandardBias;
    LONG DaylightBias;
    SYSTEMTIME StandardDate;
    SYSTEMTIME DaylightDate;
} TZI, * PTZI;

CRegKey RegKey;
CString regKey = _T("SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones");
int idx = 0;
bool bMoreKeys = true;
bool bSuccess = true;
while (bMoreKeys && bSuccess) {
    CString sSubkeyName;
    DWORD nLength = 200;
    LPTSTR sBuffer = sSubkeyName.GetBufferSetLength(nLength);
    FILETIME ftLastWriteTime;
    bMoreKeys = (RegKey.EnumKey(idx, sBuffer, & nLength, & ftLastWriteTime) == ERROR_SUCCESS);
    sSubkeyName.ReleaseBuffer();
    if (!bMoreKeys) {
        bMoreKeys = false;
        break;
    }
    CString sSubKeyPath = regKey + _T("\\") + sSubkeyName;
    CRegKey subKey;
    if (subKey.Open(HKEY_LOCAL_MACHINE, sSubKeyPath, KEY_READ) != ERROR_SUCCESS) {
        //LOG_ERROR
        bSuccess = false;
        break;
    }
    // Get the display name
    CString sDispName;
    DWORD nDispBufferLength = 1000;
    LPTSTR sDispBuffer = sDispName.GetBufferSetLength(nDispBufferLength);
    if (subKey.QueryStringValue(_T("Display"), sDispBuffer, & nDispBufferLength) != ERROR_SUCCESS) {
        //LOG_ERROR
        bSuccess = false;
        break;
    }
    sDispName.ReleaseBuffer();
    // Get the Bias (for later sorting);
    TZI tzi;
    nLength = sizeof(tzi);
    if (subKey.QueryBinaryValue(_T("TZI"), & tzi, & nLength) != ERROR_SUCCESS && nLength != sizeof(tzi)) {
        //LOG_ERROR
        bSuccess = false;
    }
    (void) subKey.Close();
    idx++;
}

But for some of the time zones for eg : argentina the return value is not error_success. On further debugging into the QueryStringValue , i found this

if ((nBytes % sizeof(TCHAR) != 0) || (pszValue[nBytes / sizeof(TCHAR) - 1] != 0)) {
    return ERROR_INVALID_DATA;
}

and when the nBytes size is 48 for any time zone display value then always error_invalid_data is returned .

To confirm this i have changed the regkey api call to

DWORD dwType = 0;
ULONG nBytes = 256 * sizeof(TCHAR);
TCHAR displayValue[256];
if (subKey.QueryValue(
    _T("Display"), & dwType, (LPBYTE) displayValue, & nBytes) == 0 && dwType == REG_SZ) {}

and i dont get the error anymore and everything is working fine . Couldnt find out any valid reason that why it occurs.Anyone has better explanation why for all the timezones with size 48 we get the invalid data error . Thanks in advance.

EDIT:

PS: In some machines everything is working well with the code mentioned above (i.e the first one) and in some other machines its not which is what really strange and with the second code everywhere i could see the code is working fine.

Answer 1

The code

CRegKey Key;
LONG nA = Key.Open(HKEY_LOCAL_MACHINE, _T("SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones\\Argentina Standard Time"), KEY_READ);
TCHAR pszValue[24];
ULONG nValueLength = _countof(pszValue);
LONG nB = Key.QueryStringValue(_T("Display"), pszValue, &nValueLength);

gets you a different error, 234, which is ERROR_MORE_DATA "More data is available."

This happens because the value itself does not fit fully into the provided buffer.

This is what you want instead:

CRegKey Key;
LONG nA = Key.Open(HKEY_LOCAL_MACHINE, _T("SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones\\Argentina Standard Time"), KEY_READ);
ULONG nValueLength = 0;
LONG nB = Key.QueryStringValue(_T("Display"), NULL, &nValueLength);
CString sValue;
if(nValueLength > 0)
{
    LONG nC = Key.QueryStringValue(_T("Display"), sValue.GetBufferSetLength(nValueLength - 1), &nValueLength);
}

Or, you need to provide large enough buffer, which you assume to be sufficient.

Answer 2

The string in the reg may not be null-terminated. Hence the following validation fails:

if ((nBytes % sizeof(TCHAR) != 0) || (pszValue[nBytes / sizeof(TCHAR) -1] != 0))
{
    return ERROR_INVALID_DATA;
}

Note that it seems to be a bug in CRegKey::QueryStringValue, RegQueryValueEx allows to read not-null-terminated strings, and it is callers responsibility to handle this case. I had come across this exact issue in the code and had to replace the call to CRegKey::QueryStringValue with my own function.