[PATCH 0/2] MR5812: odbccp32: SQLWritePrivateProfileStringW check for existing DSN first
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5812
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- 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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5812
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5812
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5812#note_72775
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?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5812#note_72776
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5812#note_72779
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5812#note_72780
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5812#note_72786
participants (3)
-
Alistair Leslie-Hughes -
Alistair Leslie-Hughes (＠alesliehughes)
-
Jinoh Kang (＠iamahuman)