Stack overflow when trying to return a unique pointer with VS2015 and addrinfoW

Question

I have done some code to retrieve addresses from a DNS, and I have a crash happening. Here is my code:

#include <ws2tcpip.h>
#include <winSock2.h>

// Need to link with Ws2_32.lib
#pragma comment(lib, "ws2_32.lib")
#include <memory>
#include <random>

bool resolveHost(const wchar_t* hostname, uint16_t port);

int main(int argc, char *argv[]) {
    WSADATA wsaData;

    WSAStartup(
      MAKEWORD(2,2),
      &wsaData
    );
    auto l = resolveHost(L"your own DNS here", 80);
}

std::unique_ptr<addrinfoW, decltype (&FreeAddrInfoW)> getAddrInfo(const wchar_t* hostname, quint16 defaultPort)
{
    addrinfoW* result;
    // zero initialization
    addrinfoW hints = {};
    hints.ai_socktype = SOCK_STREAM;
    wchar_t port[9];
    int error = swprintf_s(port, sizeof (port), L"%hu", defaultPort);
    if(error == -1)
    {
        printf("Invalid call to swprintf_s");
        return std::unique_ptr<addrinfoW, decltype (&FreeAddrInfoW)>{nullptr, &FreeAddrInfoW};
    }
    /* resolve the domain name into a list of addresses */
    error = GetAddrInfoW(hostname, port, &hints, &result);
    if (error != 0)
    {
        wprintf(gai_strerror(error));
        return std::unique_ptr<addrinfoW, decltype (&FreeAddrInfoW)>{nullptr, &FreeAddrInfoW};
    }
// here, when I return, I have a stack overflow
    return std::move(std::unique_ptr<addrinfoW, decltype (&FreeAddrInfoW)>{result, &FreeAddrInfoW});
}

bool resolveHost(const wchar_t hostname, uint16_t port)
{
    auto guard {getAddrInfo(hostname, port)};
    return true;
}

When getAddrInfo returns, I have a stack overflow with VS 2015 (I don't have another version), and I don't understand why. Can someone help me?

user7860670 · Accepted Answer · 2019-04-12T22:26:39.870

2

You pass array size in bytes instead of array elements count in swprintf_s which may cause stack corruption. In debug mode it fills the rest of the buffer after terminating null with garbage marker 0xFE. Note that there is an appropriate overload for swprintf_s deducing size of the c-style array so there is no need to pass it explicitly.

Also there should be no std::move( in the last return statement.

edited Apr 12 '19 at 22:26

answered Apr 12 '19 at 16:44

user7860670

35,849
4
58
84

IOW, `sizeof(port)` is `sizeof(wchar_t)*9=18`, so you are telling `swprintf_s()` it is OK to write up to 18 characters into the `port` buffer, but it can only hold 9 characters. But, I don't see how this can cause a stack corruption since `"%hu"` would produce only 5 formatted characters + a null terminator, which is well within `port`'s allocated memory. `swprintf_s()` doesn't null out unused characters, unlike `wcsncpy_s()` for instance. But VTT is correct, you should use the overloaded `swprintf_s()` that deduces the buffer size for you: `int error = swprintf_s(port, L"%hu", defaultPort);` – Remy Lebeau Apr 12 '19 at 21:37
1

@RemyLebeau In debug mode it fills remaining part (after terminating null) of the buffer with 0xFE. – user7860670 Apr 12 '19 at 21:45
Good to know, thanks, That should be included in your answer – Remy Lebeau Apr 12 '19 at 21:48

Stack overflow when trying to return a unique pointer with VS2015 and addrinfoW

1 Answers1