From 89375a261b92493cc9e6a07959d48e9c62dd06de Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Thu, 5 Mar 2026 10:02:05 -0800 Subject: [PATCH 1/2] Fix MAVLink message bounds validation vulnerabilities Harden MAVLink message handlers against malicious or malformed payloads that could trigger out-of-bounds memory access. ImageProtocolManager (GHSA-v5rc-wh3c-c4cw): - Validate DATA_TRANSMISSION_HANDSHAKE fields (size, payload, packets) before allocating the image buffer - Enforce 1 MB upper bound on image size - Reject payload values exceeding ENCAPSULATED_DATA data[253] array size - Pre-allocate image buffer to declared size instead of growing via unchecked indexed writes - Replace byte-by-byte copy loop with bounds-clamped memcpy - Cast seqnr to uint32_t before multiplication to prevent overflow Vehicle (LOG_DATA): - Add bounds check on log.count against sizeof(log.data) before emitting the signal, preventing downstream consumers from reading past the 90-byte data array FTPManager (FILE_TRANSFER_PROTOCOL): - Validate hdr.size against sizeof(request->data) at the single message entry point, protecting all downstream handlers (burst read, list directory, fill missing blocks) from reading past the 239-byte data array Fixes: GHSA-v5rc-wh3c-c4cw --- src/MAVLink/ImageProtocolManager.cc | 34 ++++++++++++++++++++++++----- src/Vehicle/FTPManager.cc | 6 +++++ src/Vehicle/Vehicle.cc | 8 +++++-- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/MAVLink/ImageProtocolManager.cc b/src/MAVLink/ImageProtocolManager.cc index 08f700139c16..2eab52b066aa 100644 --- a/src/MAVLink/ImageProtocolManager.cc +++ b/src/MAVLink/ImageProtocolManager.cc @@ -10,6 +10,8 @@ #include "ImageProtocolManager.h" #include "QGCLoggingCategory.h" +#include + QGC_LOGGING_CATEGORY(ImageProtocolManagerLog, "qgc.mavlink.imageprotocolmanager") ImageProtocolManager::ImageProtocolManager(QObject *parent) @@ -58,6 +60,27 @@ void ImageProtocolManager::mavlinkMessageReceived(const mavlink_message_t &messa _imageBytes.clear(); mavlink_msg_data_transmission_handshake_decode(&message, &_imageHandshake); qCDebug(ImageProtocolManagerLog) << QStringLiteral("DATA_TRANSMISSION_HANDSHAKE: type(%1) width(%2) height (%3)").arg(_imageHandshake.type).arg(_imageHandshake.width).arg(_imageHandshake.height); + + // Validate handshake fields to prevent out-of-bounds writes on subsequent ENCAPSULATED_DATA + static constexpr uint32_t kMaxImageSize = 1u * 1024u * 1024u; // 1 MB upper bound (optical flow images are typically small grayscale frames) + if (_imageHandshake.size == 0 || _imageHandshake.payload == 0 || _imageHandshake.packets == 0) { + qCWarning(ImageProtocolManagerLog) << "DATA_TRANSMISSION_HANDSHAKE: Invalid field(s) - size:" << _imageHandshake.size + << "payload:" << _imageHandshake.payload << "packets:" << _imageHandshake.packets; + _imageHandshake = {}; + break; + } + if (_imageHandshake.size > kMaxImageSize) { + qCWarning(ImageProtocolManagerLog) << "DATA_TRANSMISSION_HANDSHAKE: Image size exceeds limit. size:" << _imageHandshake.size; + _imageHandshake = {}; + break; + } + if (_imageHandshake.payload > sizeof(mavlink_encapsulated_data_t::data)) { + qCWarning(ImageProtocolManagerLog) << "DATA_TRANSMISSION_HANDSHAKE: payload exceeds ENCAPSULATED_DATA data field size. payload:" << _imageHandshake.payload; + _imageHandshake = {}; + break; + } + + _imageBytes.resize(_imageHandshake.size, '\0'); break; } case MAVLINK_MSG_ID_ENCAPSULATED_DATA: @@ -70,16 +93,17 @@ void ImageProtocolManager::mavlinkMessageReceived(const mavlink_message_t &messa mavlink_encapsulated_data_t encapsulatedData; mavlink_msg_encapsulated_data_decode(&message, &encapsulatedData); - uint32_t bytePosition = encapsulatedData.seqnr * _imageHandshake.payload; + const uint32_t bytePosition = static_cast(encapsulatedData.seqnr) * _imageHandshake.payload; if (bytePosition >= _imageHandshake.size) { qCWarning(ImageProtocolManagerLog) << "ENCAPSULATED_DATA: seqnr is past end of image size. seqnr:" << encapsulatedData.seqnr << "_imageHandshake.size:" << _imageHandshake.size; break; } - for (uint8_t i = 0; i < _imageHandshake.payload; i++) { - _imageBytes[bytePosition] = encapsulatedData.data[i]; - bytePosition++; - } + // Clamp the number of bytes to copy so we never write past the declared image size + const uint32_t bytesRemaining = _imageHandshake.size - bytePosition; + const uint32_t bytesToCopy = qMin(static_cast(_imageHandshake.payload), bytesRemaining); + + (void) memcpy(_imageBytes.data() + bytePosition, encapsulatedData.data, bytesToCopy); // We use the packets field to track completion _imageHandshake.packets--; diff --git a/src/Vehicle/FTPManager.cc b/src/Vehicle/FTPManager.cc index 156a8c3883ca..c06f8f4b7da3 100644 --- a/src/Vehicle/FTPManager.cc +++ b/src/Vehicle/FTPManager.cc @@ -237,6 +237,12 @@ void FTPManager::_mavlinkMessageReceived(const mavlink_message_t& message) MavlinkFTP::Request* request = (MavlinkFTP::Request*)&data.payload[0]; + // Clamp hdr.size to the actual data array bounds to prevent over-reads + if (request->hdr.size > sizeof(request->data)) { + qCWarning(FTPManagerLog) << "_mavlinkMessageReceived: hdr.size exceeds data array, discarding." << request->hdr.size; + return; + } + // Ignore old/reordered packets (handle wrap-around properly) uint16_t actualIncomingSeqNumber = request->hdr.seqNumber; if ((uint16_t)((_expectedIncomingSeqNumber - 1) - actualIncomingSeqNumber) < (std::numeric_limits::max()/2)) { diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc index ba6a7be316db..a16cae12c7a2 100644 --- a/src/Vehicle/Vehicle.cc +++ b/src/Vehicle/Vehicle.cc @@ -605,7 +605,7 @@ void Vehicle::_mavlinkMessageReceived(LinkInterface* link, mavlink_message_t mes mavlink_serial_control_t ser; mavlink_msg_serial_control_decode(&message, &ser); if (static_cast(ser.count) > sizeof(ser.data)) { - qWarning() << "Invalid count for SERIAL_CONTROL, discarding." << ser.count; + qCWarning(VehicleLog) << "Invalid count for SERIAL_CONTROL, discarding." << ser.count; } else { emit mavlinkSerialControl(ser.device, ser.flags, ser.timeout, ser.baudrate, QByteArray(reinterpret_cast(ser.data), ser.count)); @@ -643,7 +643,11 @@ void Vehicle::_mavlinkMessageReceived(LinkInterface* link, mavlink_message_t mes { mavlink_log_data_t log{}; mavlink_msg_log_data_decode(&message, &log); - emit logData(log.ofs, log.id, log.count, log.data); + if (static_cast(log.count) > sizeof(log.data)) { + qCWarning(VehicleLog) << "Invalid count for LOG_DATA, discarding." << log.count; + } else { + emit logData(log.ofs, log.id, log.count, log.data); + } break; } case MAVLINK_MSG_ID_MESSAGE_INTERVAL: From 399c861c0d29740acad70cfd022d7f2ab64e1f16 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Thu, 5 Mar 2026 19:53:24 -0800 Subject: [PATCH 2/2] Update to available Mac gstreamer --- .gitignore | 1 + .../VideoReceiver/GStreamer/gstqml6gl/CMakeLists.txt | 4 ++-- tools/setup/install-dependencies-osx.sh | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 7e1d4fa4161e..b83c8f55edf5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ .idea/ .vscode/ .cache/ +.ccache/ cmake-build-*/ out/ libs/lib/Frameworks/GStreamer.framework diff --git a/src/VideoManager/VideoReceiver/GStreamer/gstqml6gl/CMakeLists.txt b/src/VideoManager/VideoReceiver/GStreamer/gstqml6gl/CMakeLists.txt index 764561118524..eec3adaf7d0e 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/gstqml6gl/CMakeLists.txt +++ b/src/VideoManager/VideoReceiver/GStreamer/gstqml6gl/CMakeLists.txt @@ -4,7 +4,7 @@ if(MACOS) # Using FindGStreamer.cmake is currently bypassed on MACOS since it doesn't work # So for now we hack in a simple hardwired setup which does work find_library(GSTREAMER_FRAMEWORK GStreamer) - set(GST_PLUGINS_VERSION 1.24.12) + set(GST_PLUGINS_VERSION 1.24.13) set(GSTREAMER_FRAMEWORK_PATH "/Library/Frameworks/GStreamer.framework") if(NOT EXISTS "${GSTREAMER_FRAMEWORK_PATH}") message(FATAL_ERROR "GStreamer.framework not found at ${GSTREAMER_FRAMEWORK_PATH}. Install GStreamer using tools/setup/install-dependencies-osx.sh script") @@ -23,7 +23,7 @@ endif() ################################################################################ if(GStreamer_VERSION VERSION_GREATER_EQUAL 1.22) - # Use Latest Revisions for each minor version: 1.20.7, 1.22.12, 1.24.12, 1.26.2 + # Use Latest Revisions for each minor version: 1.20.7, 1.22.12, 1.24.13, 1.26.2 string(REPLACE "." ";" GST_VERSION_LIST ${GStreamer_VERSION}) list(GET GST_VERSION_LIST 0 GST_VERSION_MAJOR) list(GET GST_VERSION_LIST 1 GST_VERSION_MINOR) diff --git a/tools/setup/install-dependencies-osx.sh b/tools/setup/install-dependencies-osx.sh index c253fbe81acb..e6e25e56ba2e 100755 --- a/tools/setup/install-dependencies-osx.sh +++ b/tools/setup/install-dependencies-osx.sh @@ -12,7 +12,7 @@ brew install cmake ninja ccache git pkgconf create-dmg # Install GStreamer GST_URL=https://gstreamer.freedesktop.org/data/pkg/osx -GST_VERSION=1.24.12 +GST_VERSION=1.24.13 GST_PKG=gstreamer-1.0-$GST_VERSION-universal.pkg GST_DEV_PKG=gstreamer-1.0-devel-$GST_VERSION-universal.pkg pushd $TMPDIR