Add ASAN and UBSAN stages to CI, fix various errors [AP-1386]

[woodfell]

Description

@swift-nav/devinfra

Add bazel config to build with ASAN, UBSAN, TSAN, or MSAN

Add CI stages to run tests with ASAN and UBSAN. The other sanitizers aren't important to run regularly given the nature of code in this repository, but they exist for consistency with other projects and to enable manually running those sanitizer if required

A couple of fixes to problems in code which were discovered by the sanitizer. No breaking changes though

API compatibility

No

API compatibility plan

No

JIRA Reference

https://swift-nav.atlassian.net/browse/AP-1386

[woodfell]

For messages with variable length arrays the legacy API generates a struct with a zero length array at the end. Something like:

struct sbp_obs_t {
  struct sbp_common_header_t header;
  struct sbp_packed_obs_t obs[0];
};

The size of this struct doesn't include the variable length array. So in this instance sizeof(SbpHeaderParams::payload) == 5 (the size of the header only) but when the handler is invoked a few lines down the payload length will be anything up to 255. This causes a buffer overflow.

[woodfell]

Changes in this template required because fields in the wire encoded byte stream rarely have the correct alignment. Eg, a 2 byte integer has an alignment of 2 (on typical systems), but the starting address of the field in the byte stream might not be properly aligned.

x86 tends to cope with this properly which is why this hasn't caused problems so far, but it is a problem detectable by one of the sanitizers. This solution adds a little helper function to memcpy a few bytes from the bytestream in to a stack variable which is guaranteed to be properly aligned, then return to the caller.

This method reduces the amount of code changes required, although it does lead to a rather annoying use of decltype a few lines down. Since this is in legacy code and will be deleted soon it seems a reasonable solution.

[woodfell]

For variable length arrays this can cause a buffer overflow.

The modern API deals with variable length arrays by generating a maximally sized array plus a companion field which indicates how many elements are valid. For example, a message containing a variable length array of uint8_t:

struct sbp_msg_sample_t {
  uint32_t some_other_field;
  uint8_t bytes[SBP_MSG_SAMPLE_BYTES_MAX];
  size_t n_bytes;
};

This function make_lesser_greater is used by the test cases for comparison functions. The generator will use this function by:

• Given some sample data from the test specs create 2 instances of the test message containing identical data
• For each field in the message spec call make_lesser_greater to purposefully introduce a different between the 2 instances
• Run a series of comparisons on both instances and validate the results

One of the tests compares the altered message instance to itself. This is meant to ensure that even with the altered field the message will compare equal to something which was altered in the same way.

The function make_lesser_greater prefers to increment a field value and only decrements it if the field is already at the maximum value for the type. In the case of the companion field for variable length arrays (n_bytes in the strawman code above) this can lead the comparison field to believe there are more elements in the variable length array than there actually are. ASAN will pick up on this describing it as a buffer overflow.

The simple solution is to alter make_lesser_greater to prefer decrementing a field value and only incrementing it if the field is at the minimum value for the type. This could lead to exactly the same problem if the test spec provides 0 elements for the variable length array, however the changes to cope with this are too intrusive to write on this PR, they can be handled separately

[sonarcloud]

Quality Gate passed for 'libsbp-c'

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[reimerix]

Awesome, thanks for adding this!