[PATCH v2 1/2] inetmib: Avoid overflows in IPv4 address comparisons.

[Francois Gouget]

The difference between two ULONGs may not fit in an int, causing
comparison errors.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
v2: Introduce DWORD_cmp() to simplify the comparisons.

compareIpAddrRow() and oidToIpForwardRow() should also do byte-swapping
which will be done in the next patch.
---
 dlls/inetmib1/main.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/dlls/inetmib1/main.c b/dlls/inetmib1/main.c
index 90da277375e..bd39abb6240 100644
--- a/dlls/inetmib1/main.c
+++ b/dlls/inetmib1/main.c
@@ -770,11 +770,15 @@ static void oidToIpAddrRow(AsnObjectIdentifier *oid, void *dst)
     row->dwAddr = oidToIpAddr(oid);
 }
 
-static int __cdecl compareIpAddrRow(const void *a, const void *b)
+static int __cdecl DWORD_cmp(DWORD a, DWORD b)
 {
-    const MIB_IPADDRROW *key = a, *value = b;
+    return a < b ? -1 : a > b ? 1 : 0; /* a substraction would overflow */
+}
 
-    return key->dwAddr - value->dwAddr;
+static int __cdecl compareIpAddrRow(const void *a, const void *b)
+{
+    const MIB_IPADDRROW *rowA = a, *rowB = b;
+    return DWORD_cmp(rowA->dwAddr, rowB->dwAddr);
 }
 
 static BOOL mib2IpAddrQuery(BYTE bPduType, SnmpVarBind *pVarBind,
@@ -865,9 +869,8 @@ static void oidToIpForwardRow(AsnObjectIdentifier *oid, void *dst)
 
 static int __cdecl compareIpForwardRow(const void *a, const void *b)
 {
-    const MIB_IPFORWARDROW *key = a, *value = b;
-
-    return key->dwForwardDest - value->dwForwardDest;
+    const MIB_IPFORWARDROW *rowA = a, *rowB = b;
+    return DWORD_cmp(rowA->dwForwardDest, rowB->dwForwardDest);
 }
 
 static BOOL mib2IpRouteQuery(BYTE bPduType, SnmpVarBind *pVarBind,
@@ -1223,13 +1226,9 @@ static void oidToUdpRow(AsnObjectIdentifier *oid, void *dst)
 
 static int __cdecl compareUdpRow(const void *a, const void *b)
 {
-    const MIB_UDPROW *key = a, *value = b;
-    int ret;
-
-    ret = ntohl(key->dwLocalAddr) - ntohl(value->dwLocalAddr);
-    if (ret == 0)
-        ret = ntohs(key->dwLocalPort) - ntohs(value->dwLocalPort);
-    return ret;
+    const MIB_UDPROW *rowA = a, *rowB = b;
+    return DWORD_cmp(ntohl(rowA->dwLocalAddr), ntohl(rowB->dwLocalAddr)) ||
+           ntohs(rowA->dwLocalPort) - ntohs(rowB->dwLocalPort);
 }
 
 static BOOL mib2UdpEntryQuery(BYTE bPduType, SnmpVarBind *pVarBind,
-- 
2.30.2

[PATCH v2 2/2] inetmib1: Fix the IPv4 address network order for comparisons.

[Francois Gouget]

compareUdpRow() was already performing the byte-swaps but
compareIpAddrRow() and compareIpForwardRow() need them too.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
v2: Introduce DWORD_cmp() to simplify the comparisons.

The byte order was fixed in compareUdpRow() for bug 52224:
https://bugs.winehq.org/show_bug.cgi?id=52224

There is no specific test for this but adding traces showed that
the other two comparison functions were also operating on the wrong
byte order.
---
 dlls/inetmib1/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/inetmib1/main.c b/dlls/inetmib1/main.c
index bd39abb6240..819f3eea2a8 100644
--- a/dlls/inetmib1/main.c
+++ b/dlls/inetmib1/main.c
@@ -778,7 +778,7 @@ static int __cdecl DWORD_cmp(DWORD a, DWORD b)
 static int __cdecl compareIpAddrRow(const void *a, const void *b)
 {
     const MIB_IPADDRROW *rowA = a, *rowB = b;
-    return DWORD_cmp(rowA->dwAddr, rowB->dwAddr);
+    return DWORD_cmp(ntohl(rowA->dwAddr), ntohl(rowB->dwAddr));
 }
 
 static BOOL mib2IpAddrQuery(BYTE bPduType, SnmpVarBind *pVarBind,
@@ -870,7 +870,7 @@ static void oidToIpForwardRow(AsnObjectIdentifier *oid, void *dst)
 static int __cdecl compareIpForwardRow(const void *a, const void *b)
 {
     const MIB_IPFORWARDROW *rowA = a, *rowB = b;
-    return DWORD_cmp(rowA->dwForwardDest, rowB->dwForwardDest);
+    return DWORD_cmp(ntohl(rowA->dwForwardDest), ntohl(rowB->dwForwardDest));
 }
 
 static BOOL mib2IpRouteQuery(BYTE bPduType, SnmpVarBind *pVarBind,
-- 
2.30.2

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=108051

Your paranoid android.


=== debian11 (32 bit report) ===

inetmib1:
main: Timeout

=== debian11 (32 bit Chinese:China report) ===

inetmib1:
main: Timeout

=== debian11 (32 bit WoW report) ===

inetmib1:
main: Timeout

=== debian11 (64 bit WoW report) ===

inetmib1:
main: Timeout