VPN-6268 - Fix ping Event Ready

[strseb]

Ugh this is an ugly one?

So the crash is happening in

memcpy(&sequence, replies[i].Data, sizeof(quint16)); 

With a EXE_BAD_ACCESS , so replies[i].Data is pointing to something wierd. Given that i cannot reproduce this, i'm trying to fix things that could theoretically have this point to a bad thing.

1. We're not cleaning up the buffer before we re-pass it to windows, mabye there is corrupt data? I'm now cleaning this up before.
2. We're casting the buffer to be an arraylike of ICMP_ECHO_REPLY, however that is not really the shape of the thing in memory.I was wondering where replies[0 ... replyCount].Data points and who owns the memory. We'll it is in our buffer! That's why the docs specifically asks us to allocate at least sizeof(ICMP_ECHO_REPLY) +WindowsPingPayloadSize + ICMP_ERR_SIZE + sizeof(IO_STATUS_BLOCK); bytes.

• This means we can represent that memory with a packed struct.
• Now given we also know how much memory we allocated for ICMP_ECHO_REPLY we can remove the for loop, given we can have only 0 or 1 Ping packages.
  -> With only 1 Possible Ping in the Struct, we know the position of the sequence in the buffer, so we can clearly just assert that the icmp ping points to that and use the data, no memcpy needed. Worst case here, it's 0 given we cleared it before causing a ping timeout but not a crash :)

[strseb]

Given we now compile with warnings as errors, my MSVC compiler complained about the static type casting.

[brizental]

Suggested change

	logger.error() << "Invalid amount of responses recieved";
	logger.error() << "Invalid amount of responses received";

[oskirby]

I filed a comment about it in the JIRA issue, but I think we should also switch out the ICMP_ECHO_REPLY struct with an ICMP_ECHO_REPLY32 as per the IcmpParseReplies documentation. It seems there is some 32/64-bit backwards compatibility issue around the Data pointer.

My guess as to the cause of the crash is that for whatever reason the Windows kernel is writing a 32-bit pointer into memory, but because we interpret the struct as a ICMP_ECHO_REPLY the client interprets the pointer as being 64-bits, and it just so happens to work fine when the top bits are zero, but crashes otherwise.

[oskirby]

Or better yet, check if the compiler macros are targeting a 64-bit system and make this an ICMP_ECHO_REPLY or ICMP_ECHO_REPLY32 as appropriate.

Changes Requested were addressed :)