Environment_WIN32U nodeIdImpl access violation in 1.13.0

[mecsco]

Describe the bug
After upgrading from 1.12.4 to 1.13.0, in Environment_WIN32U.cpp the EnvironmentImpl::nodeIdImpl function throws an access violation when making the second call to GetAdaptersInfo if the first call has returned 111 (ERROR_BUFFER_OVERFLOW)

This appears to have been introduced in #4328 (#4336, cfc9ce3).

Expected behavior
Same behaviour as 1.12.4

Please add relevant environment information:

• Windows 10 64 bit
• POCO 1.13.0
• Visual Studio 2022
• x86 / Win32 project configuration

[andrewauclair]

Can you send the length returned by the first GetAdaptersInfo? It should be a multiple of the struct size from what I understood while making this change.

That'll be the value in len after the call.

[mecsco]

len is 3840, and sizeof(IP_ADAPTER_INFO) is 648

[andrewauclair]

I believe this is some sort of packing issue. On the default packing the IP_ADAPTER_INFO struct is 704 bytes for me, GetAdaptersInfo returns 2816 (Win10 x64), which is a proper 4 multiple.

winnt.h contains the following assert, which verifies that the default packing option is used. Unfortunately it appears that this assert doesn't happen for other headers, such as iphlpapi.h in this case.

static_assert(__alignof(LARGE_INTEGER) == 8, "Windows headers require the default packing option. Changing this can lead to memory corruption."
    " This diagnostic can be disabled by building with WINDOWS_IGNORE_PACKING_MISMATCH defined.");

648 appears to be the correct size for x86. I can't find a way to get that size for x64. Even forcing #pragma pack(push, 1) before including the header for IP_ADAPTER_INFO results in a minimum of 676.

[mecsco]

Apologies, I've clarified the environment in my original comment - the project configuration is indeed x86

[andrewauclair]

According to this page on IP_ADAPTER_INFO, there's an extra step to properly use it on 32 bit platforms.

To properly use the IP_ADAPTER_INFO structure on a 32-bit platform, define _USE_32BIT_TIME_T (use -D _USE_32BIT_TIME_T as an option, for example) when compiling the application to force the time_t datatype to a 4-byte datatype.

I'm not sure if _USE_32BIT_TIME_T is something Poco could/should set. But if you set it in your program, I think it will fix the problem.

For me:

without _USE_32BIT_TIME_T: IP_ADAPTER_INFO size: 648, len: 2560, count: 3.95
with _USE_32BIT_TIME_T: IP_ADAPTER_INFO size: 640, len: 2560, count: 4

[aleks-f]

I'm not sure if _USE_32BIT_TIME_T is something Poco could/should set. But if you set it in your program, I think it will fix the problem.

It should be set for this case only, and then unset (if it wasn't set to start with) to prevent it from causing some unforeseen problems elsewhere

[matejk]

Page https://learn.microsoft.com/en-us/windows/win32/api/iptypes/ns-iptypes-ip_adapter_info contains remarks:

The IP_ADAPTER_INFO structure is limited to IPv4 information about a particular network adapter on the local computer. The IP_ADAPTER_INFO structure is retrieved by calling the GetAdaptersInfo function.
When using Visual Studio 2005 and later, the time_t datatype defaults to an 8-byte datatype, not the 4-byte datatype used for the LeaseObtained and LeaseExpires members on a 32-bit platform. To properly use the IP_ADAPTER_INFO structure on a 32-bit platform, define _USE_32BIT_TIME_T (use -D _USE_32BIT_TIME_T as an option, for example) when compiling the application to force the time_t datatype to a 4-byte datatype.
For use on Windows XP and later, the IP_ADAPTER_ADDRESSES structure contains both IPv4 and IPv6 information. The GetAdaptersAddresses function retrieves IPv4 and IPv6 adapter information.

Proper solution in the long term might be to use GetAdaptersAddresses instead of GetAdaptersInfo.

[matejk]

I wanted to fix this problem in PR #4405 to set compile-time define to a single file when compiling on 32-bit Windows.

That solution is suboptimal and hacky. Function GetAdaptersInfo can be replaced with newer function GetAdaptersAddresses that does not have time_t size issues. (https://learn.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses).

@andrewauclair, would you be willing to rewrite the function Windows variant of EnvironmentImpl::nodeIdImpl to use GetAdaptersAddresses?

[andrewauclair]

@matejk Sure I'll check it out this week. It's sounding like that's the better long-term solution here. I do agree with Microsoft on this. As of a few days ago there's less than 14 years left to 32 bit time_t and it shouldn't be used anymore.

Edit: 2038 - 2024 is 14 years, not 4. I've been sick for a few days and it's not helping my brain right now.

[andrewauclair]

Is Environment::nodeId something that might be called often? I've been experimenting with GetAdaptersAddresses and it requests a nearly 20k byte buffer, compared to GetAdaptersInfo which requests a buffer of 3,500 bytes.

[matejk]

We don't really have an option, IMO. The temporary buffer is released each time anyway.

Are you concerned about memory usage?

20kB shall not be a problem on a Windows machine I think.

[andrewauclair]

You're right, it probably doesn't matter, and we have to do it.

Hoping to put together a PR this weekend. Through a lot of experimenting, I learned that we can't allocate an array of the struct, we need to allocate an array of bytes like before. GetAdaptersAddresses creates a linked list structure and its size varies depending on the version of Windows. That shouldn't matter for Poco because the info nodeIdImpl is interested in is in all versions.

[matejk]

Example on MS web site can be easily adapted, I think.