From: "Zhipeng Zhao" Subject: Re:Re: crypt32: Add parameter validation. Message-Id: <4b60fe14.2474.1731240b910.Coremail.near2see@163.com> Date: Fri, 3 Jul 2020 09:18:18 +0800 (CST) In-Reply-To: <4cc6d01e-8192-0058-6c07-44c9d9f7a0fe@codeweavers.com> References: <460f781b.5f2c.1730e164288.Coremail.near2see@163.com> <4cc6d01e-8192-0058-6c07-44c9d9f7a0fe@codeweavers.com> Thanks your review, after a bit more test, I found the behavior of these func is consistent of Windows'. The problem I got need to be solved in some elsewhere. At 2020-07-02 14:33:25, "Zhiyi Zhang" wrote: On 7/2/20 1:53 PM, Zhipeng Zhao wrote: 0001-crypt32-Add-parameter-validation.patch From 63c44d4282ecdbb233bf3c26463336d08ca04ea3 Mon Sep 17 00:00:00 2001 From: Zhipeng Zhao Date: Thu, 2 Jul 2020 13:47:26 +0800 Subject: [PATCH] crypt32: Add parameter validation. Signed-off-by: Zhipeng Zhao --- dlls/crypt32/msg.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/crypt32/msg.c b/dlls/crypt32/msg.c index f591a5e72e..42cb9d0e31 100644 --- a/dlls/crypt32/msg.c +++ b/dlls/crypt32/msg.c @@ -3660,6 +3660,12 @@ BOOL WINAPI CryptMsgClose(HCRYPTMSG hCryptMsg) BOOL WINAPI CryptMsgUpdate(HCRYPTMSG hCryptMsg, const BYTE *pbData, DWORD cbData, BOOL fFinal) { + if (!hCryptMsg) + { + SetLastError(E_INVALIDARG); + return FALSE; + } + CryptMsgBase *msg = hCryptMsg; This makes the msg declaration no longer at the beginning of a block. See https://wiki.winehq.org/Developer_Hints#Using_only_C89-compliant_code What application needs this change? If hCryptMsg really needs to be checked against NULL, there are many more functions that make use of the handle. Maybe you should add a helper for it. And please add tests for this change. Thanks, Zhiyi TRACE("(%p, %p, %d, %d)\n", hCryptMsg, pbData, cbData, fFinal); @@ -3670,6 +3676,12 @@ BOOL WINAPI CryptMsgUpdate(HCRYPTMSG hCryptMsg, const BYTE *pbData, BOOL WINAPI CryptMsgGetParam(HCRYPTMSG hCryptMsg, DWORD dwParamType, DWORD dwIndex, void *pvData, DWORD *pcbData) { + if (!hCryptMsg) + { + SetLastError(E_INVALIDARG); + return FALSE; + } + CryptMsgBase *msg = hCryptMsg; TRACE("(%p, %d, %d, %p, %p)\n", hCryptMsg, dwParamType, dwIndex, @@ -3680,6 +3692,12 @@ BOOL WINAPI CryptMsgGetParam(HCRYPTMSG hCryptMsg, DWORD dwParamType, BOOL WINAPI CryptMsgControl(HCRYPTMSG hCryptMsg, DWORD dwFlags, DWORD dwCtrlType, const void *pvCtrlPara) { + if (!hCryptMsg) + { + SetLastError(E_INVALIDARG); + return FALSE; + } + CryptMsgBase *msg = hCryptMsg; TRACE("(%p, %08x, %d, %p)\n", hCryptMsg, dwFlags, dwCtrlType, Thanks your review, after a bit more test, I found the behavior of these func is consistent of Windows'. The problem I got need to be solved in some elsewhere.

At 2020-07-02 14:33:25, "Zhiyi Zhang" <zzhang@codeweavers.com> wrote:



On 7/2/20 1:53 PM, Zhipeng Zhao wrote:

0001-crypt32-Add-parameter-validation.patch
From 63c44d4282ecdbb233bf3c26463336d08ca04ea3 Mon Sep 17 00:00:00 2001
From: Zhipeng Zhao <zhaozhipeng@uniontech.com>
Date: Thu, 2 Jul 2020 13:47:26 +0800
Subject: [PATCH] crypt32: Add parameter validation.

Signed-off-by: Zhipeng Zhao <zhaozhipeng@uniontech.com>
---
 dlls/crypt32/msg.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/dlls/crypt32/msg.c b/dlls/crypt32/msg.c
index f591a5e72e..42cb9d0e31 100644
--- a/dlls/crypt32/msg.c
+++ b/dlls/crypt32/msg.c
@@ -3660,6 +3660,12 @@ BOOL WINAPI CryptMsgClose(HCRYPTMSG hCryptMsg)
 BOOL WINAPI CryptMsgUpdate(HCRYPTMSG hCryptMsg, const BYTE *pbData,
  DWORD cbData, BOOL fFinal)
 {
+    if (!hCryptMsg)
+    {
+        SetLastError(E_INVALIDARG);
+        return FALSE;
+    }
+
     CryptMsgBase *msg = hCryptMsg;
This makes the msg declaration no longer at the beginning of a block.
See https://wiki.winehq.org/Developer_Hints#Using_only_C89-compliant_code

What application needs this change? If hCryptMsg really needs to be checked
against NULL, there are many more functions that make use of the handle.
Maybe you should add a helper for it.

And please add tests for this change.

Thanks,
Zhiyi
 
 
     TRACE("(%p, %p, %d, %d)\n", hCryptMsg, pbData, cbData, fFinal);
@@ -3670,6 +3676,12 @@ BOOL WINAPI CryptMsgUpdate(HCRYPTMSG hCryptMsg, const BYTE *pbData,
 BOOL WINAPI CryptMsgGetParam(HCRYPTMSG hCryptMsg, DWORD dwParamType,
  DWORD dwIndex, void *pvData, DWORD *pcbData)
 {
+    if (!hCryptMsg)
+    {
+        SetLastError(E_INVALIDARG);
+        return FALSE;
+    }
+
     CryptMsgBase *msg = hCryptMsg;
 
     TRACE("(%p, %d, %d, %p, %p)\n", hCryptMsg, dwParamType, dwIndex,
@@ -3680,6 +3692,12 @@ BOOL WINAPI CryptMsgGetParam(HCRYPTMSG hCryptMsg, DWORD dwParamType,
 BOOL WINAPI CryptMsgControl(HCRYPTMSG hCryptMsg, DWORD dwFlags,
  DWORD dwCtrlType, const void *pvCtrlPara)
 {
+    if (!hCryptMsg)
+    {
+        SetLastError(E_INVALIDARG);
+        return FALSE;
+    }
+
     CryptMsgBase *msg = hCryptMsg;
 
     TRACE("(%p, %08x, %d, %p)\n", hCryptMsg, dwFlags, dwCtrlType,