[PATCH 0/2] MR3105: Add a few additional nullchecks (Coverity)

[Fabian Maurer (＠DarkShadow44)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3105

[PATCH 1/2] mmdevapi: Add nullcheck for parameter that can be null (Coverity)

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

"out" got checked for null earlier, so we should do it here as well

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
---
 dlls/mmdevapi/client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index a0a2b225c0b..6721b3c267e 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -480,7 +480,7 @@ HRESULT WINAPI client_IsFormatSupported(IAudioClient3 *iface, AUDCLNT_SHAREMODE
 
     WINE_UNIX_CALL(is_format_supported, ¶ms);
 
-    if (params.result == S_FALSE)
+    if (params.result == S_FALSE && out)
         *out = ¶ms.fmt_out->Format;
     else
         CoTaskMemFree(params.fmt_out);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3105

[PATCH 2/2] advpack: Add nullcheck for parameter that can be null (Coverity)

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

---
 dlls/advpack/install.c       |  5 ++++-
 dlls/advpack/tests/install.c | 13 +++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/dlls/advpack/install.c b/dlls/advpack/install.c
index deaa82d6d98..6f6a00dd4b0 100644
--- a/dlls/advpack/install.c
+++ b/dlls/advpack/install.c
@@ -591,7 +591,7 @@ HRESULT WINAPI ExecuteCabA(HWND hwnd, CABINFOA* pCab, LPVOID pReserved)
 
     TRACE("(%p, %p, %p)\n", hwnd, pCab, pReserved);
 
-    if (!pCab)
+    if (!pCab || !pCab->pszInf)
         return E_INVALIDARG;
 
     if (pCab->pszCab)
@@ -643,6 +643,9 @@ HRESULT WINAPI ExecuteCabW(HWND hwnd, CABINFOW* pCab, LPVOID pReserved)
 
     TRACE("(%p, %p, %p)\n", hwnd, pCab, pReserved);
 
+    if (!pCab->pszInf)
+        return E_INVALIDARG;
+
     ZeroMemory(&info, sizeof(ADVInfo));
 
     if ((pCab->pszCab && *pCab->pszCab) && (pCab->pszInf && *pCab->pszInf) && *pCab->szSrcPath)
diff --git a/dlls/advpack/tests/install.c b/dlls/advpack/tests/install.c
index bdf02c5a397..fa1d68c5bef 100644
--- a/dlls/advpack/tests/install.c
+++ b/dlls/advpack/tests/install.c
@@ -262,6 +262,18 @@ static void test_LaunchINFSectionEx(void)
     DeleteFileA("test.inf");
 }
 
+static void test_ExecuteCab(void)
+{
+    HRESULT hr;
+    CABINFOA cabinfoa = {0};
+    CABINFOW cabinfow = {0};
+
+    hr = ExecuteCabA(0, &cabinfoa, 0);
+    ok(hr == E_INVALIDARG, "Got %lx\n", hr);
+    hr = ExecuteCabW(0, &cabinfow, 0);
+    ok(hr == E_INVALIDARG, "Got %lx\n", hr);
+}
+
 START_TEST(install)
 {
     DWORD len;
@@ -289,6 +301,7 @@ START_TEST(install)
     test_RunSetupCommand();
     test_LaunchINFSection();
     test_LaunchINFSectionEx();
+    test_ExecuteCab();
 
     FreeLibrary(hAdvPack);
     SetCurrentDirectoryA(prev_path);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3105

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/mmdevapi/client.c:
> WINE_UNIX_CALL(is_format_supported, ¶ms);
>
> -    if (params.result == S_FALSE)
> +    if (params.result == S_FALSE && out)

I believe this is unnecessary as the driver should only return `S_FALSE` in the `AUDCLNT_SHAREMODE_SHARED` case where `out` will be non-NULL.  However, it would be a good idea to move the parameter checks that are currently done in the first few lines of each driver into `mmdevapi` - @davidebeatrici something to add to the list (not really a priority though).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3105#note_36237