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/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: 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