[PATCH 0/1] MR7357: rpcrt4: Avoid unsigned overflows in buffer validation.

[Jacek Caban (＠jacek)]

Fixes Clang 20 `-Wtautological-compare` warnings. Based on patch by William Horvath.

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

[PATCH 1/1] rpcrt4: Avoid unsigned overflows in buffer validation.

[Jacek Caban]

From: Jacek Caban <jacek(a)codeweavers.com>

Fixes Clang 20 -Wtautological-compare warnings. Based on patch by William Horvath.
---
 dlls/rpcrt4/ndr_marshall.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/dlls/rpcrt4/ndr_marshall.c b/dlls/rpcrt4/ndr_marshall.c
index c68eb0ee57e..24b6f8125a9 100644
--- a/dlls/rpcrt4/ndr_marshall.c
+++ b/dlls/rpcrt4/ndr_marshall.c
@@ -711,8 +711,8 @@ static inline ULONG safe_multiply(ULONG a, ULONG b)
 
 static inline void safe_buffer_increment(MIDL_STUB_MESSAGE *pStubMsg, ULONG size)
 {
-    if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */
-        (pStubMsg->Buffer + size > (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength))
+    unsigned char *buffer_end = (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength;
+    if (pStubMsg->Buffer > buffer_end || buffer_end - pStubMsg->Buffer < size)
         RpcRaiseException(RPC_X_BAD_STUB_DATA);
     pStubMsg->Buffer += size;
 }
@@ -732,8 +732,7 @@ static inline void safe_buffer_length_increment(MIDL_STUB_MESSAGE *pStubMsg, ULO
  * to do so */
 static inline void safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, ULONG size)
 {
-    if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */
-        (pStubMsg->Buffer + size > pStubMsg->BufferEnd))
+    if (pStubMsg->BufferEnd < pStubMsg->Buffer || pStubMsg->BufferEnd - pStubMsg->Buffer < size)
     {
         ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n",
             pStubMsg->Buffer, pStubMsg->BufferEnd, size);
@@ -748,8 +747,8 @@ static inline void safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, U
 /* copies data to the buffer, checking that there is enough space to do so */
 static inline void safe_copy_to_buffer(MIDL_STUB_MESSAGE *pStubMsg, const void *p, ULONG size)
 {
-    if ((pStubMsg->Buffer + size < pStubMsg->Buffer) || /* integer overflow of pStubMsg->Buffer */
-        (pStubMsg->Buffer + size > (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength))
+    unsigned char *buffer_end = (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength;
+    if (pStubMsg->Buffer > buffer_end || buffer_end - pStubMsg->Buffer < size)
     {
         ERR("buffer overflow - Buffer = %p, BufferEnd = %p, size = %lu\n",
             pStubMsg->Buffer, (unsigned char *)pStubMsg->RpcMsg->Buffer + pStubMsg->BufferLength,
@@ -767,8 +766,7 @@ static void validate_string_data(MIDL_STUB_MESSAGE *pStubMsg, ULONG bufsize, ULO
     ULONG i;
 
     /* verify the buffer is safe to access */
-    if ((pStubMsg->Buffer + bufsize < pStubMsg->Buffer) ||
-        (pStubMsg->Buffer + bufsize > pStubMsg->BufferEnd))
+    if (pStubMsg->BufferEnd < pStubMsg->Buffer || pStubMsg->BufferEnd - pStubMsg->Buffer < bufsize)
     {
         ERR("bufsize 0x%lx exceeded buffer end %p of buffer %p\n", bufsize,
             pStubMsg->BufferEnd, pStubMsg->Buffer);
-- 
GitLab

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

[PATCH 0/1] MR7357: rpcrt4: Avoid unsigned overflows in buffer validation. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7357

[Alexandre Julliard (＠julliard)]

This could use a helper function to avoid duplicating the checks.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7357#note_95141