Found by ASan.
-- v3: iphlpapi: Give GetBestRoute2 a SOCKETADDR_INET.
From: Yuxuan Shui yshui@codeweavers.com
In this call chain: GetBestInterface -> GetBestInterfaceEx -> GetBestRoute2, we cast a socketaddr_in * (16 bytes) is to SOCKETADDR_INET * (28 bytes) before passing it to GetBestRoute2, but GetBestRoute2 will access all of the 28 bytes, leading to out-of-bound access.
Found by ASan. --- dlls/iphlpapi/iphlpapi_main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 83c6e8b53b8..e5fa9a74cde 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -1369,10 +1369,10 @@ ULONG WINAPI DECLSPEC_HOTPATCH GetAdaptersAddresses( ULONG family, ULONG flags, */ DWORD WINAPI GetBestInterface(IPAddr dwDestAddr, PDWORD pdwBestIfIndex) { - struct sockaddr_in sa_in; + SOCKADDR_INET sa_in; memset(&sa_in, 0, sizeof(sa_in)); - sa_in.sin_family = AF_INET; - sa_in.sin_addr.S_un.S_addr = dwDestAddr; + sa_in.si_family = AF_INET; + sa_in.Ipv4.sin_addr.S_un.S_addr = dwDestAddr; return GetBestInterfaceEx((struct sockaddr *)&sa_in, pdwBestIfIndex); }
This merge request was approved by Paul Gofman.
TL;DL; Note GetBestRoute2 even zeroes output bestaddress in full and that is backed by the tests. Whether GetBestRoute2 accesses src and dst in full or shorter part in case of ipv4 is currently not tested, but I think it more likely does on Windows (and if I didn't have to fixup dst address in the concerned place I would just do 'connect( s, (struct sockaddr *)dst, sizeof(*dst) )' and that access would happen in ws2_32 where it would copy the address for DeviceIoControl). That 'SOCKADDR_INET *' passed for dst, not a void pointer with explicit length, so from general sense it is totally valid to access the whole structure, and the implementation would naturally do it if it would, e. g., passing the address to nsiproxy or ws2_32 without interpreting here.
This may be true of GetBestRoute2(), but I'm not sure that the same argument extends to GetBestInterfaceEx(). It takes a `struct sockaddr`, and it would seem legitimate to pass it that instead of a `SOCKADDR_INET`.
On Fri Oct 3 16:09:53 2025 +0000, Alexandre Julliard wrote:
TL;DL; Note GetBestRoute2 even zeroes output bestaddress in full and
that is backed by the tests. Whether GetBestRoute2 accesses src and dst in full or shorter part in case of ipv4 is currently not tested, but I think it more likely does on Windows (and if I didn't have to fixup dst address in the concerned place I would just do 'connect( s, (struct sockaddr *)dst, sizeof(*dst) )' and that access would happen in ws2_32 where it would copy the address for DeviceIoControl). That 'SOCKADDR_INET *' passed for dst, not a void pointer with explicit length, so from general sense it is totally valid to access the whole structure, and the implementation would naturally do it if it would, e. g., passing the address to nsiproxy or ws2_32 without interpreting here. This may be true of GetBestRoute2(), but I'm not sure that the same argument extends to GetBestInterfaceEx(). It takes a `struct sockaddr`, and it would seem legitimate to pass it that instead of a `SOCKADDR_INET`.
Uh, sorry, I missed here that 'struct sockaddr' is actually not a generic address but just ipv4 address and shorter than SOCKADDR_INET. What contributes to confusion is that GetBestInterfaceEx() actually supports ipv6 addresses (that is in tests). MS docs specify the argument as 'sockaddr *' [1] which assumes different handling based on protocol, while the actual headers definition (also in Windows SDK) has 'struct sockaddr *'.
I guess then we must leave GetBestInterface() alone and instead fill local SOCKADDR_INET in GetBestInterfaceEx() to pass to GetBestRoute2().
1. https://learn.microsoft.com/en-us/windows/win32/winsock/sockaddr-2
-
Alexandre Julliard (＠julliard) -
Paul Gofman (＠gofman)
-
Yuxuan Shui
-
Yuxuan Shui (＠yshui)