usdot-fhwa-stol / carma-messenger

CARMA Messenger will be a scaled down version of CARMA Platform for a non-automated vehicle to participate in Cooperative Driving Automation (CDA). Doxygen Source Code Documentation: https://usdot-fhwa-stol.github.io/documentation/carma-messenger/
14 stars 11 forks source link

[cpp_message] Fix segmentation fault when encoding SDSM messages #217

Closed adamlm closed 7 months ago

adamlm commented 7 months ago

PR Details

Description

This PR fixes a segmentation fault during SDSM encoding caused by trying to copy the message's source ID. The TemporaryID.msg's id field gets implemented as a std::vector<std::uint8_t>, which is not guaranteed to have any elements. The SDSM encoding function assumes that the SensorDataSharingMessage.msg's source_id field has 4 elements.

This PR adds a size check before copying the data.

Related GitHub Issue

Related Jira Key

Closes CDAR-775

Motivation and Context

Bug fix

How Has This Been Tested?

Manually in integration testing

Types of changes

Checklist:

adamlm commented 7 months ago

I think it won't hurt to have this guard. However, this source_id seems required for the SDSM standard. How is it possible that SDSM received have empty field here? Could it be the SDSM being generated itself is not up to standard?

The received SDSM is malformed because the source_id field does not adhere to the Standard.

MishkaMN commented 7 months ago

I think it won't hurt to have this guard. However, this source_id seems required for the SDSM standard. How is it possible that SDSM received have empty field here? Could it be the SDSM being generated itself is not up to standard?

The received SDSM is malformed because the source_id field does not adhere to the Standard.

Ah yeah it does look like the node forgot to set the source_id. Could we make a PR for that?

https://github.com/usdot-fhwa-stol/carma-platform/blob/release/lavida/carma_cooperative_perception/src/msg_conversion.cpp#L543-L572

we are also not setting msgCount, equipmentType, refPosXYConf image