C++ - Access violation when constructing ICMP packet

Question

I am working on a ping tool and I am consistently getting an access violation around my sending buffer while calculating the ICMP checksum when using a packet size of 45041 or greater (including ICMP header). Any packet with size 45040 or below throws no error and properly transmits with correct checksum. Abbreviated code is below; the access violation occurs when dereferencing the buffer within the while loop in the checksum function on the first iteration.

typedef struct ICMPHeader 
{
  BYTE type;          // ICMP packet type
  BYTE code;          // Type sub code
  USHORT checksum;
  USHORT id;
  USHORT seq;
} ICMPHeader;

typedef struct echoRequest
{
  ICMPHeader icmpHead;
  char *data;
} EchoRequest;

// ...
EchoRequest *sendBuffer = new EchoRequest();
sendBuffer->data = new char[packetSize];

memset((void *)sendBuffer->data, 0xfa, packetSize);

sendBuffer->icmpHead.checksum = ipChecksum((USHORT *)sendBuffer, 
                                     packetSize + sizeof(sendBuffer->icmpHead));
// ...

// checksum function
USHORT ipChecksum(USHORT *buffer, unsigned long size)
{
  unsigned long cksum = 0;

  while (size > 1) 
  {
    cksum += *buffer++;
    size -= sizeof(USHORT);
  }

  if (size)
    cksum += *(UCHAR *)buffer;

  cksum = (cksum >> 16) + (cksum & 0xffff);
  cksum += (cksum >> 16);

  return (USHORT)(~cksum);
}

Any ideas as to why this is happening?

Exact error wording: Unhandled exception at 0x009C2582 in PingProject.exe: 0xC0000005: Access violation reading location 0x004D5000.

Using Visual Studio Professional 2012 with platform toolset v100 for .NET 4.0

Answer 1

Your ipChecksum function expects a pointer to the data it's supposed to checksum, not a pointer to a structure that contains a pointer to the data to checksum. So first it checksums icmpHead, which is good. But then it checksums the pointer to data, which makes no sense. And then it checksums off the end of the EchoRequest structure.

Answer 2

If you want this code to be interpreted as c++ by the readers you need to fix some things.

• memset really?

• use reinterpret_cast to convert one pointer type to another.

• it's generally considered a much better practice to use size_t instead of unsigned long

• use smart pointers instead.

• use static_cast to convert ulong to ushort.

• USHORT is not guaranteed to be 16 bits. Use a different type instead.

Edit: You're waaaay above MTU. Keep your packets under 1k bytes. IEEE 802.3 expects 1492 though this value might vary.