3

Im trying to read a registry using the winapi and c++.

The code runs, but the result is not the contents of the registry After a hexdump is just 0xCD repeated over and over. (So, as if the data hasnt been modified by RegQueryValueEx, and is just the result of the malloc) I tried running as admin too, with no luck.

This is the code im using:

HKEY hKey;
if (RegOpenKeyEx(HKEY_CURRENT_USER, "Software\\Microsoft\\Windows\\Shell\\Bags\\1\\Desktop", 0, KEY_ALL_ACCESS, &hKey) != ERROR_SUCCESS)
    return;

//Read & save
DWORD BufferSize = TOTALBYTES;
DWORD cbData;
DWORD dwRet;

LPBYTE data = (LPBYTE)malloc(BufferSize);
cbData = BufferSize;

DWORD type = REG_BINARY;

dwRet = RegQueryValueEx(hKey, "IconLayouts", NULL, &type, data, &cbData);

while (dwRet == ERROR_MORE_DATA) {

    BufferSize += BYTEINCREMENT;
    data = (LPBYTE)realloc(data, BufferSize);
    cbData = BufferSize;

    dwRet = RegQueryValueEx(hKey, "IconLayouts", NULL, &type, data, &cbData);
}

if (dwRet == ERROR_SUCCESS)
{
    //Write current registry to a file
    std::ofstream currentRegistryFile(DIRECTORY + currentDesktop + ".bin");
    if (!currentRegistryFile) {
        log(currentDesktop + " file couldn't be opened.");
        return;
    }
    for (int i = 0; i < cbData; i++)
        currentRegistryFile << (data)[cbData];
}
else
    log("Couldnt read registry");


//Close registry
RegCloseKey(hKey);
  • 1) ask for `KEY_READ` instead `KEY_ALL_ACCESS`. 2) change `while` to `do while` loop - with this you need call `RegQueryValueEx` only once in scr code. as and `malloc`. 3) you not free `data`. 4.) need not `BufferSize += BYTEINCREMENT` but `BufferSize = cbData` - `RegQueryValueEx` return to you exactly buffer size need. – RbMm Jan 28 '18 at 20:51
  • and your error `(data)[cbData];` you read after buffer end. instead `data[i]` – RbMm Jan 28 '18 at 20:55
  • @RbMm It’s interesting that Microsoft’s documentation mentions cbData returning the required size, but their example code still does this += BYTEINCREMENT thing. Have to send feedback about it. – Sami Kuhmonen Jan 28 '18 at 20:57
  • @SamiKuhmonen - very not the best example code. – RbMm Jan 28 '18 at 21:01
  • ***0xCD repeated over and over*** 0xCD = uninitialized heap memory. https://stackoverflow.com/a/127404/487892 – drescherjm Jan 29 '18 at 02:40

1 Answers1

2

Your saving code is the problem. It’s actually accessing the array out of bounds:

for (int i = 0; i < cbData; i++)
    currentRegistryFile << (data)[cbData];

Note you’re indexing data with constant value of cbData and not loop variable i. Change that.

Sami Kuhmonen
  • 30,146
  • 9
  • 61
  • 74
  • Why didn't I notice that! Another change the OP should make is replace `std::ofstream currentRegistryFile(DIRECTORY + currentDesktop + ".bin");` with `std::ofstream currentRegistryFile(DIRECTORY + currentDesktop + ".bin", std::ios_base::binary);` since this is clearly binary data, – john Jan 28 '18 at 21:03