AppJailLauncher icon indicating copy to clipboard operation
AppJailLauncher copied to clipboard

Subtle bug: InetNtop passed incorrect buffer size

Open ariccio opened this issue 7 years ago • 0 comments

I'm screwing around with /analyze, and it's picked up a couple of issues for AppJailLauncher. This one is unlikely to cause any trouble, but it still violates the InetNtop API contract, and technically could cause a buffer overrun.

At line 419 in AppJailLauncher.cpp you call it with sizeof(clientIpAddr) for the StringBufSize parameter:

					InetNtop(
					((struct sockaddr_in *) &clientAddr)->sin_family,
					(PVOID)&((struct sockaddr_in *) &clientAddr)->sin_addr,
					clientIpAddr,
					sizeof(clientIpAddr)
					)

...where clientIpAddr is declared as _TCHAR clientIpAddr[64] ...and the docs for StringBufSize say:

StringBufSize [in]
On input, the length, in characters, of the buffer pointed to by the pStringBuf parameter.

...so all is fine if you're compiling for a target where sizeof(TCHAR) == 1), but not where it's a typedef wchar_t _TCHAR.

It's unlikely to cause any trouble because of this note in the docs:

pStringBuf [out]
A pointer to a buffer in which to store the NULL-terminated string representation of the IP address.
For an IPv4 address, this buffer should be large enough to hold at least 16 characters.
For an IPv6 address, this buffer should be large enough to hold at least 46 characters.

ariccio avatar Nov 29 '16 23:11 ariccio