ArduPilot / ArduPilot/pymavlink

Undefined Behavior in mavlink_helpers.h: Left Shift of Negative Value

Open
#321 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
728
Forks
739
Avg merge
4d 10h
Merged PRs (30d)
7

Description

The clang [UndefinedBehaviorSanitizer](https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html) flagged a line in the mavlink C library v1 in [mavlink_helpers.h](https://github.com/ArduPilot/pymavlink/blob/f55377066e5f5dba2c63b7a5d116454b1b0ce303/generator/C/include_v1.0/mavlink_helpers.h):

```C
if (status->msg_received == MAVLINK_FRAMING_BAD_CRC) {
/*
the CRC came out wrong. We now need to overwrite the
msg CRC with the one on the wire so that if the
caller decides to forward the message anyway that
mavlink_msg_to_send_buffer() won't overwrite the
checksum
*/
r_message->checksum = _MAV_PAYLOAD(rxmsg)[status->packet_idx] |(_MAV_PAYLOAD(rxmsg)[status->packet_idx+1]<<8);
}
```

Here, the intention is to overwrite `r_message->checksum` with a 2-btye checksum from `_MAV_PAYLOAD`. The second char is left-shifted by 8 bits. [Left-shifting negative values results in undefined behavior in C](https://stackoverflow.com/questions/3784996/why-does-left-shift-operation-invoke-undefined-behaviour-when-the-left-side-oper).

The byte types could be changed to `uint8_t`, or it may be sufficient to reinterpret the values as `uint16_t`:

```
r_message->checksum = *reinterpret_cast(_MAV_PAYLOAD(rxmsg)[status->packet_idx]);
```

I did not check other versions of mavlink C, but the problem may exist there as well.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in generator/C/include_v1.0/mavlink_helpers.h at the checksum assignment reported by Clang UBSan. Check the other MAVLink C versions mentioned in the issue, then verify that the checksum is assembled without undefined left-shift behavior and that UBSan no longer reports the case.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.