[PATCH 0/2] MR5812: odbccp32: SQLWritePrivateProfileStringW check for existing DSN first

[Alistair Leslie-Hughes (＠alesliehughes)]

If a DSN is System wide, we need to write to the HKEY_LOCAL_MACHINE
part of the registry not HKEY_CURRENT_USER which it's currently doing.

[PATCH 1/2] odbccp32: Support System wide ODBC DSN keys

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/odbccp32/odbccp32.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
index 95bfb90781e..f1e6cb810c4 100644
--- a/dlls/odbccp32/odbccp32.c
+++ b/dlls/odbccp32/odbccp32.c
@@ -713,20 +713,23 @@ BOOL WINAPI SQLGetInstalledDrivers(char *buf, WORD size, WORD *sizeout)

static HKEY get_privateprofile_sectionkey(LPCWSTR section, LPCWSTR filename)
 {
-    HKEY hkey, hkeyfilename, hkeysection;
+    HKEY hkeysection;
     LONG ret;
+    WCHAR regpath[256];

-    if (RegOpenKeyW(HKEY_CURRENT_USER, odbcW, &hkey))
-        return NULL;
+    wcscpy(regpath, L"Software\ODBC\");
+    wcscat(regpath, filename);
+    wcscat(regpath, L"\");
+    wcscat(regpath, section);
+
+    if ((ret = RegOpenKeyW(HKEY_CURRENT_USER, regpath, &hkeysection)) != ERROR_SUCCESS)
+    {
+        ret = RegOpenKeyW(HKEY_LOCAL_MACHINE, regpath, &hkeysection);
+    }

-    ret = RegOpenKeyW(hkey, filename, &hkeyfilename);
-    RegCloseKey(hkey);
     if (ret)
         return NULL;

-    ret = RegOpenKeyW(hkeyfilename, section, &hkeysection);
-    RegCloseKey(hkeyfilename);
-
     return ret ? NULL : hkeysection;
 }

[PATCH 2/2] odbccp32: SQLWritePrivateProfileStringW check for existing DSN first

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

If a DSN is System wide, we need to write to the HKEY_LOCAL_MACHINE
part of the registry not HKEY_CURRENT_USER which it's currently doing.
---
 dlls/odbccp32/odbccp32.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
index f1e6cb810c4..df940951f22 100644
--- a/dlls/odbccp32/odbccp32.c
+++ b/dlls/odbccp32/odbccp32.c
@@ -1807,6 +1807,7 @@ BOOL WINAPI SQLWritePrivateProfileStringW(LPCWSTR lpszSection, LPCWSTR lpszEntry
     static const WCHAR empty[] = {0};
     LONG ret;
     HKEY hkey;
+    WCHAR regpath[256];

clear_errors();
     TRACE("%s %s %s %s\n", debugstr_w(lpszSection), debugstr_w(lpszEntry),
@@ -1818,7 +1819,26 @@ BOOL WINAPI SQLWritePrivateProfileStringW(LPCWSTR lpszSection, LPCWSTR lpszEntry
         return FALSE;
     }

-    if ((ret = RegCreateKeyW(HKEY_CURRENT_USER, odbcW, &hkey)) == ERROR_SUCCESS)
+    wcscpy(regpath, L"Software\ODBC\");
+    wcscat(regpath, lpszFilename);
+    wcscat(regpath, L"\");
+    wcscat(regpath, lpszSection);
+
+    /* Check an existing key first before writing a new one */
+    if ((ret = RegOpenKeyW(HKEY_CURRENT_USER, regpath, &hkey)) != ERROR_SUCCESS)
+    {
+        ret = RegOpenKeyW(HKEY_LOCAL_MACHINE, regpath, &hkey);
+    }
+
+    if (ret == ERROR_SUCCESS)
+    {
+        if(lpszString)
+            ret = RegSetValueExW(hkey, lpszEntry, 0, REG_SZ, (BYTE*)lpszString, (lstrlenW(lpszString)+1)*sizeof(WCHAR));
+        else
+            ret = RegSetValueExW(hkey, lpszEntry, 0, REG_SZ, (BYTE*)empty, sizeof(empty));
+        RegCloseKey(hkey);
+    }
+    else if ((ret = RegCreateKeyW(HKEY_CURRENT_USER, odbcW, &hkey)) == ERROR_SUCCESS)
     {
          HKEY hkeyfilename;

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/odbccp32/odbccp32.c:
> static HKEY get_privateprofile_sectionkey(LPCWSTR section, LPCWSTR filename)
>  {
> - HKEY hkey, hkeyfilename, hkeysection;
> - HKEY hkeysection;
> LONG ret;
> - WCHAR regpath[256];
> - if (RegOpenKeyW(HKEY_CURRENT_USER, odbcW, &hkey))
> -
>    return NULL;
> - wcscpy(regpath, L"Software\ODBC\");
> - wcscat(regpath, filename);
> - wcscat(regpath, L"\");
> - wcscat(regpath, section);

This looks very easy to overrun.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/odbccp32/odbccp32.c:
> - wcscat(regpath, L"\");
> - wcscat(regpath, lpszSection);
> -
> - /* Check an existing key first before writing a new one */
> - if ((ret = RegOpenKeyW(HKEY_CURRENT_USER, regpath, &hkey)) != ERROR_SUCCESS)
> - {
> -
>    ret = RegOpenKeyW(HKEY_LOCAL_MACHINE, regpath, &hkey);
> - }
> -
> - if (ret == ERROR_SUCCESS)
> - {
> -
>    if(lpszString)
> -
>        ret = RegSetValueExW(hkey, lpszEntry, 0, REG_SZ, (BYTE*)lpszString, (lstrlenW(lpszString)+1)*sizeof(WCHAR));
> -
>    else
> -
>        ret = RegSetValueExW(hkey, lpszEntry, 0, REG_SZ, (BYTE*)empty, sizeof(empty));
> -
>    RegCloseKey(hkey);

If HKCU key didn't exist. We'd overwrite the HKLM key. This isn't possible on Windows w/o admin privileges. Are we sure we want to do this?

[Alistair Leslie-Hughes (＠alesliehughes)]

On Sat Jun  8 10:02:45 2024 +0000, Jinoh Kang wrote:
> This looks very easy to overrun the buffer.

I'll fix this one up.

[Alistair Leslie-Hughes (＠alesliehughes)]

On Sat Jun  8 10:02:23 2024 +0000, Jinoh Kang wrote:
> If HKCU key didn't exist. We'd overwrite the HKLM key. This isn't
> possible on Windows w/o admin privileges. Are we sure we want to do this?

For installers, this needs to be done (which will have admin privileges).

I'm not sure exactly what occurs on windows, I'll have to do some more digging. If we write to the wrong location, wine than has an incomplete DSN and fails to find the driver it needs when queried.

[Jinoh Kang (＠iamahuman)]

On Sat Jun  8 12:21:17 2024 +0000, Alistair Leslie-Hughes wrote:
> For installers, this needs to be done (which will have admin privileges).
> I'm not sure exactly what occurs on windows, I'll have to do some more
> digging. If we write to the wrong location, wine than has an incomplete
> DSN and fails to find the driver it needs when queried.

Ah, thanks for clarifying.