2
\$\begingroup\$

This seems to work on some version of windows and acts weird on others. This is unstable and redundant.

How can I improve this in a way that it has the same expected behavior through every windows version starting from Windows XP to 10.

When the application starts it checks if a unique ID already exists. If it does, it uses that unique ID for some other task. If it does not, then it creates a unique ID and then uses it for some other task.

To note, this is the definition of buf and len:

unsigned char *buf, size_t len,

Here is the code that needs reviewing:

    HKEY hKey = nullptr;
    UUID uuid;
    DWORD dwDisp;
    DWORD dwUUIDSize = sizeof(uuid);
    unsigned char dwUUID[16];

    std::vector<unsigned char> login_buf(len + dwUUIDSize);

    for (size_t i = 0; i < len; ++i) login_buf[i] = buf[i];

    if (RegCreateKeyEx(HKEY_CURRENT_USER, TEXT("Software\\Mirinae\\Khan_LI"), 0, NULL, 0, KEY_WRITE | KEY_READ, NULL, &hKey, &dwDisp) == ERROR_SUCCESS) {
        if (dwDisp == REG_CREATED_NEW_KEY) 
        {
            UuidCreate(&uuid);

            dwUUID[0] = (uuid.Data1 >> 0) & 0xFF;
            dwUUID[1] = (uuid.Data1 >> 8) & 0xFF;
            dwUUID[2] = (uuid.Data1 >> 16) & 0xFF;
            dwUUID[3] = (uuid.Data1 >> 24) & 0xFF;
            dwUUID[4] = (uuid.Data2 >> 0) & 0xFF;
            dwUUID[5] = (uuid.Data2 >> 8) & 0xFF;
            dwUUID[6] = (uuid.Data3 >> 0) & 0xFF;
            dwUUID[7] = (uuid.Data3 >> 8) & 0xFF;
            dwUUID[8] = uuid.Data4[0];
            dwUUID[9] = uuid.Data4[1];
            dwUUID[10] = uuid.Data4[2];
            dwUUID[11] = uuid.Data4[3];
            dwUUID[12] = uuid.Data4[4];
            dwUUID[13] = uuid.Data4[5];
            dwUUID[14] = uuid.Data4[6];
            dwUUID[15] = uuid.Data4[7];

            RegSetValueEx(hKey, TEXT("keyinfo"), 0, REG_BINARY, (LPBYTE)dwUUID, dwUUIDSize);

            for (size_t i = 0; i < dwUUIDSize; i++) login_buf.push_back(dwUUID[i]);
        }
        else {
            if (RegQueryValueEx(hKey, TEXT("keyinfo"), 0, 0, (LPBYTE)dwUUID, &dwUUIDSize) == ERROR_SUCCESS)
            {
                for (size_t i = 0; i < dwUUIDSize; i++) login_buf.push_back(dwUUID[i]);
            }
            else {
                UuidCreate(&uuid);

                dwUUID[0] = (uuid.Data1 >> 0) & 0xFF;
                dwUUID[1] = (uuid.Data1 >> 8) & 0xFF;
                dwUUID[2] = (uuid.Data1 >> 16) & 0xFF;
                dwUUID[3] = (uuid.Data1 >> 24) & 0xFF;
                dwUUID[4] = (uuid.Data2 >> 0) & 0xFF;
                dwUUID[5] = (uuid.Data2 >> 8) & 0xFF;
                dwUUID[6] = (uuid.Data3 >> 0) & 0xFF;
                dwUUID[7] = (uuid.Data3 >> 8) & 0xFF;
                dwUUID[8] = uuid.Data4[0];
                dwUUID[9] = uuid.Data4[1];
                dwUUID[10] = uuid.Data4[2];
                dwUUID[11] = uuid.Data4[3];
                dwUUID[12] = uuid.Data4[4];
                dwUUID[13] = uuid.Data4[5];
                dwUUID[14] = uuid.Data4[6];
                dwUUID[15] = uuid.Data4[7];

                RegSetValueEx(hKey, TEXT("keyinfo"), 0, REG_BINARY, (LPBYTE)dwUUID, dwUUIDSize);

                for (size_t i = 0; i < dwUUIDSize; i++) login_buf.push_back(dwUUID[i]);
            }
        }

        RegCloseKey(hKey);

        // add username
        char strbuf[500];
        int strlen;

        char *cname = nullptr;
        size_t cnameLen = 0;
        _dupenv_s(&cname, &cnameLen, "COMPUTERNAME");
        char *uname = nullptr;
        size_t unameLen = 0;
        _dupenv_s(&uname, &unameLen, "USERNAME");

        // add os info
        if (IsWindowsXPOrGreater())       strlen = sprintf_s(strbuf, "%s|%s|WindowsXP", cname, uname);
        if (IsWindowsXPSP1OrGreater())    strlen = sprintf_s(strbuf, "%s|%s|WindowsXP-SP1", cname, uname);
        if (IsWindowsXPSP2OrGreater())    strlen = sprintf_s(strbuf, "%s|%s|WindowsXP-SP2", cname, uname);
        if (IsWindowsXPSP3OrGreater())    strlen = sprintf_s(strbuf, "%s|%s|WindowsXP-SP3", cname, uname);
        if (IsWindowsVistaOrGreater())    strlen = sprintf_s(strbuf, "%s|%s|WindowsVista", cname, uname);
        if (IsWindowsVistaSP1OrGreater()) strlen = sprintf_s(strbuf, "%s|%s|WindowsVista-SP1", cname, uname);
        if (IsWindowsVistaSP2OrGreater()) strlen = sprintf_s(strbuf, "%s|%s|WindowsVista-SP2", cname, uname);
        if (IsWindows7OrGreater())        strlen = sprintf_s(strbuf, "%s|%s|Windows7", cname, uname);
        if (IsWindows7SP1OrGreater())     strlen = sprintf_s(strbuf, "%s|%s|Windows7-SP1", cname, uname);
        if (IsWindows8OrGreater())        strlen = sprintf_s(strbuf, "%s|%s|Windows8/10", cname, uname);
        if (IsWindows8Point1OrGreater())  strlen = sprintf_s(strbuf, "%s|%s|Windows8.1/10", cname, uname);

        login_buf.push_back((strlen >> 0) & 0xFF);
        login_buf.push_back((strlen >> 8) & 0xFF);
        for (size_t i = 0; i < strlen; i++) login_buf.push_back(strbuf[i]);

        buf = &login_buf[0];
        len = login_buf.size();

After everything, buf and len gets passed to another function that sends the data through the network.

\$\endgroup\$
  • 1
    \$\begingroup\$ Can you explain in a few sentences what you mean by "ID" and "unique"? i.e. give a bit of context? \$\endgroup\$ – einpoklum Aug 13 '17 at 11:02
  • \$\begingroup\$ @einpoklum I'm just trying to have a unique identifier for each application. So I can use it as a session_id when it connects to the server. Just like how a browser uses a cookie. \$\endgroup\$ – majidarif Aug 13 '17 at 11:54
  • \$\begingroup\$ What, to you, qualifies as an "identifier"? In what sense it is "unique"? What makes two applications different? And - if what you need is something to use as a session ID, why not just use your process ID? And if you need something like a cookie, why are you always regenerating it? \$\endgroup\$ – einpoklum Aug 13 '17 at 11:58
  • \$\begingroup\$ @einpoklum I'm only regenerating it when the registry value doesn't exist. If it does I will just use the existing value. 2 applications on the same machine are considered same. The identifier is unique to the machine and not actually the application which is why I used the registry to store the value. \$\endgroup\$ – majidarif Aug 13 '17 at 12:09
1
\$\begingroup\$

The name dwUUID appears to be ill-chosen, as it is bytes rather than double words.

Just after UuidCreate(), you populate dwUUID first with the style of shifting and masking, then with the style of subscripting. Pick one style and stick with it. Also, it looks like this copy-n-pasted code belongs in a helper function, called by an if clause and also by an else clause.

When you add os info, you call sprintf_s() a great many times in the usual (modern) case. You could replace the ifs that make that call with ifs that repeatedly assign fmt_string, followed by just a single formatting call.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.