Skip to content

Fix alignment issues in oob_build_api_payload

Carson Riker requested to merge fennewald/librist:fix-oob-in-oob into master

Currently, there are two issues in oob_build_api_payload, specifically with IP checksum calculation

In tools/oob_shared.c:84 oob_build_api_payload:

// buffer is a char *
ip->iph_chksum = csum((unsigned short *)buffer, total_len);

Unaligned cast

Casting from char * to unsigned short * is unsound, as the compiler may algin the char to an odd address.

In practice, all the places this is called, buffer happens to be aligned in a way that does not cause issues, but this is a coincidence, and cannot be relied on. The solution is to correctly align buffer, by specifying it's type as uint16_t *

Accidental unit conversion

oob_build_api_payload calculates total_len to represent the length of the full message, in bytes. However, when used as an argument to csum, it is treated as a number of 16-bit words. This results in the checksum reading twice the region it should, which could SIGSEGV and die.

Again, in current usages, this does not occur, because:

  • buffer is always 500 bytes
  • message is always <= 200 bytes
  • total_len is always <= 220 bytes
  • max OOB read is 220 * 2 - 1, A.K.A, buffer[439], which is < 500

The solution to this is straightforward:

  1. Update oob_build_api_payload to take buffer as a uint16_t * communicating alignment requirements up-front
  2. Update callsites to use buffers of uint16_ts, over the old char.
  3. Correctly supply length to csum

Merge request reports

Loading