[PATCH v2 1/2] inetmib: Avoid overflows in IPv4 address comparisons.
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
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
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
participants (2)
-
Francois Gouget -
Marvin