fix(security): backport security fixes to release/1.17 - #26741
Merged
mrpollo merged 7 commits intoMar 13, 2026
Merged
mrpollo merged 7 commits into
mrpollo merged 7 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Reject replies with length >= sizeof(BSTPacket) to prevent OOB read in CRC calculation. Clamp dev_name_len to buffer size to prevent OOB write during null termination. Signed-off-by: Ramon Roche <mrpollo@gmail.com>
Add bounds checking in the CAN frame assembly loop to prevent a buffer overflow when copying payloads into the Tattu12SBatteryMessage struct. A crafted CAN frame with a corrupt payload_size could write past the 48-byte struct boundary. Also guard against payload_size of 0 which would cause an unsigned integer underflow on the size_t subtraction. Fixes GHSA-wxwm-xmx9-hr32 Signed-off-by: Ramon Roche <mrpollo@gmail.com>
Add _validatePath() that rejects paths containing ".." components, preventing directory traversal outside the FTP root directory. Applied to all FTP operation handlers (list, open, remove, truncate, rename, mkdir, rmdir, CRC32). Fixes GHSA-fh32-qxj9-x32f, GHSA-pm28-2j4f-8jxv Signed-off-by: Ramon Roche <mrpollo@gmail.com>
Remove the old MAVLINK_FTP_UNIT_TEST infrastructure that has been dead code for years (not enabled in any board config). This includes: - src/modules/mavlink/mavlink_tests/ directory (test suite, CMakeLists) - All #ifdef MAVLINK_FTP_UNIT_TEST blocks in mavlink_ftp.cpp - set_unittest_worker() callback mechanism in mavlink_ftp.h - Conditional uAvionix include in mavlink_bridge_header.h The test suite will be ported to GTest as a follow-up. Ref: #26738 Signed-off-by: Ramon Roche <mrpollo@gmail.com>
…tions Use logical OR (||) instead of AND (&&) in _workWrite() and _workBurst() session validation, matching the correct logic already used in _workRead() and _workTerminate(). The AND operator allowed operations to proceed with an invalid session ID as long as a valid file descriptor existed. Signed-off-by: Ramon Roche <mrpollo@gmail.com>
Reject Zenoh payloads that exceed the expected uORB topic size plus CDR header (4 bytes), or that are too small to contain a valid CDR header. This prevents a stack overflow from crafted network input where z_bytes_len(payload) controls a VLA allocation. Fixes GHSA-69g4-hcqf-j45p Signed-off-by: Ramon Roche <mrpollo@gmail.com>
The mavlink_tests module was deleted in 1009268 but several references were left behind, breaking builds on all targets. Removed: - CMakeLists.txt: add_subdirectory(mavlink_tests) - mavlink_ftp.cpp: #include of deleted mavlink_ftp_test.h - mavlink_ftp.h: MavlinkFtpTest forward decl and friend class - posix-configs/SITL/init/test/test_mavlink: dead init script - sitl_tests.cmake: sitl-mavlink CTest target - install-voxl.sh: px4-mavlink_tests symlink Ref: #26738 Signed-off-by: Ramon Roche <mrpollo@gmail.com>
mrpollo
marked this pull request as ready for review
March 13, 2026 17:50
Copy link
Copy Markdown
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 272 byte (0.01 %)]FILE SIZE VM SIZE -------------- -------------- +0.0% +272 +0.0% +272 .text [NEW] +72 [NEW] +72 MavlinkFTP::_validatePath() +0.0% +72 +0.0% +72 [section .text] [NEW] +60 [NEW] +60 CSWTCH.1951 [NEW] +44 [NEW] +44 CSWTCH.2741 +18% +24 +18% +24 MavlinkFTP::_workRename() +14% +20 +14% +20 px4::bst::BST::probe() +14% +16 +14% +16 MavlinkFTP::_workCalcFileCRC32() +4.3% +16 +4.3% +16 MavlinkFTP::_workList() [NEW] +14 [NEW] +14 CSWTCH.3481 [NEW] +14 [NEW] +14 CSWTCH.3482 +13% +12 +13% +12 MavlinkFTP::_workCreateDirectory() +7.1% +12 +7.1% +12 MavlinkFTP::_workOpen() +18% +12 +18% +12 MavlinkFTP::_workRemoveDirectory() +14% +12 +14% +12 MavlinkFTP::_workRemoveFile() +2.9% +12 +2.9% +12 MavlinkFTP::_workTruncateFile() +2.2% +12 +2.2% +12 MavlinkReceiver::CheckHeartbeats() [DEL] -14 [DEL] -14 CSWTCH.3472 [DEL] -14 [DEL] -14 CSWTCH.3473 -100.0% -20 -100.0% -20 [15 Others] [DEL] -44 [DEL] -44 CSWTCH.2732 [DEL] -60 [DEL] -60 CSWTCH.1943 +0.0% +56 [ = ] 0 .debug_abbrev +0.0% +8 [ = ] 0 .debug_aranges +0.0% +24 [ = ] 0 .debug_frame +0.0% +323 [ = ] 0 .debug_info +0.0% +269 [ = ] 0 .debug_line +133% +4 [ = ] 0 [Unmapped] +0.0% +265 [ = ] 0 [section .debug_line] +0.0% +49 [ = ] 0 .debug_loclists -0.0% -6 [ = ] 0 .debug_rnglists [NEW] +3 [ = ] 0 [Unmapped] -0.0% -9 [ = ] 0 [section .debug_rnglists] -0.0% -91 [ = ] 0 .debug_str -1.2% -3 [ = ] 0 .shstrtab +0.0% +35 [ = ] 0 .strtab [DEL] -12 [ = ] 0 CSWTCH.1943 [NEW] +12 [ = ] 0 CSWTCH.1951 [DEL] -12 [ = ] 0 CSWTCH.2732 [NEW] +12 [ = ] 0 CSWTCH.2741 [DEL] -24 [ = ] 0 CSWTCH.3472 [DEL] -24 [ = ] 0 CSWTCH.3473 [NEW] +24 [ = ] 0 CSWTCH.3481 [NEW] +24 [ = ] 0 CSWTCH.3482 [NEW] +35 [ = ] 0 MavlinkFTP::_validatePath() +48% +16 [ = ] 0 ___Z12get_orb_meta6ORB_ID_veneer -38.1% -16 [ = ] 0 __nxsem_freeholder_veneer +0.0% +48 [ = ] 0 .symtab [DEL] -32 [ = ] 0 CSWTCH.1943 [NEW] +32 [ = ] 0 CSWTCH.1951 [DEL] -32 [ = ] 0 CSWTCH.2732 [NEW] +32 [ = ] 0 CSWTCH.2741 [DEL] -48 [ = ] 0 CSWTCH.3472 [DEL] -48 [ = ] 0 CSWTCH.3473 [NEW] +48 [ = ] 0 CSWTCH.3481 [NEW] +48 [ = ] 0 CSWTCH.3482 +100% +16 [ = ] 0 MavlinkFTP::_data_as_cstring() [NEW] +48 [ = ] 0 MavlinkFTP::_validatePath() -50.0% -16 [ = ] 0 MavlinkFTP::_workCalcFileCRC32() +33% +16 [ = ] 0 MavlinkFTP::_workOpen() -33.3% -16 [ = ] 0 MavlinkFTP::_workReset() -50.0% -16 [ = ] 0 MavlinkStreamAutopilotStateForGimbalDevice::send() +33% +16 [ = ] 0 MavlinkStreamBatteryInfo::send() -50.0% -16 [ = ] 0 MavlinkULog::start_ack_received() +33% +16 [ = ] 0 MavlinkULog::~MavlinkULog() +50% +16 [ = ] 0 McAutotuneAttitudeControl::updateParamsImpl() -140.0% -32 [ = ] 0 [1 Others] -0.1% -16 [ = ] 0 [section .symtab] +67% +32 [ = ] 0 ___Z12get_orb_meta6ORB_ID_veneer -2.4% -272 [ = ] 0 [Unmapped] +0.0% +712 +0.0% +272 TOTAL FILE SIZE VM SIZE -------------- -------------- +0.0% +224 +0.0% +224 .text [NEW] +72 [NEW] +72 MavlinkFTP::_validatePath() [NEW] +60 [NEW] +60 CSWTCH.1951 [NEW] +44 [NEW] +44 CSWTCH.2741 +0.0% +44 +0.0% +44 [section .text] +18% +24 +18% +24 MavlinkFTP::_workRename() +14% +16 +14% +16 MavlinkFTP::_workCalcFileCRC32() +4.3% +16 +4.3% +16 MavlinkFTP::_workList() [NEW] +14 [NEW] +14 CSWTCH.3481 [NEW] +14 [NEW] +14 CSWTCH.3482 +13% +12 +13% +12 MavlinkFTP::_workCreateDirectory() +7.1% +12 +7.1% +12 MavlinkFTP::_workOpen() +18% +12 +18% +12 MavlinkFTP::_workRemoveDirectory() +14% +12 +14% +12 MavlinkFTP::_workRemoveFile() +2.9% +12 +2.9% +12 MavlinkFTP::_workTruncateFile() +2.2% +12 +2.2% +12 MavlinkReceiver::CheckHeartbeats() -0.8% -8 -0.8% -8 MavlinkReceiver::handle_message_hil_state_quaternion() -100.0% -12 -100.0% -12 [13 Others] [DEL] -14 [DEL] -14 CSWTCH.3472 [DEL] -14 [DEL] -14 CSWTCH.3473 [DEL] -44 [DEL] -44 CSWTCH.2732 [DEL] -60 [DEL] -60 CSWTCH.1943 +0.0% +56 [ = ] 0 .debug_abbrev +0.0% +8 [ = ] 0 .debug_aranges +0.0% +24 [ = ] 0 .debug_frame +0.0% +276 [ = ] 0 .debug_info +0.0% +244 [ = ] 0 .debug_line [NEW] +7 [ = ] 0 [Unmapped] +0.0% +237 [ = ] 0 [section .debug_line] +0.0% +38 [ = ] 0 .debug_loclists -0.0% -11 [ = ] 0 .debug_rnglists -66.7% -2 [ = ] 0 [Unmapped] -0.0% -9 [ = ] 0 [section .debug_rnglists] -0.0% -91 [ = ] 0 .debug_str +0.4% +1 [ = ] 0 .shstrtab +0.0% +35 [ = ] 0 .strtab [DEL] -12 [ = ] 0 CSWTCH.1943 [NEW] +12 [ = ] 0 CSWTCH.1951 [DEL] -12 [ = ] 0 CSWTCH.2732 [NEW] +12 [ = ] 0 CSWTCH.2741 [DEL] -24 [ = ] 0 CSWTCH.3472 [DEL] -24 [ = ] 0 CSWTCH.3473 [NEW] +24 [ = ] 0 CSWTCH.3481 [NEW] +24 [ = ] 0 CSWTCH.3482 [NEW] +35 [ = ] 0 MavlinkFTP::_validatePath() +0.0% +48 [ = ] 0 .symtab [DEL] -32 [ = ] 0 CSWTCH.1943 [NEW] +32 [ = ] 0 CSWTCH.1951 [DEL] -32 [ = ] 0 CSWTCH.2732 [NEW] +32 [ = ] 0 CSWTCH.2741 [DEL] -48 [ = ] 0 CSWTCH.3472 [DEL] -48 [ = ] 0 CSWTCH.3473 [NEW] +48 [ = ] 0 CSWTCH.3481 [NEW] +48 [ = ] 0 CSWTCH.3482 +100% +16 [ = ] 0 MavlinkFTP::_data_as_cstring() [NEW] +48 [ = ] 0 MavlinkFTP::_validatePath() -50.0% -16 [ = ] 0 MavlinkFTP::_workCalcFileCRC32() +33% +16 [ = ] 0 MavlinkFTP::_workOpen() -33.3% -16 [ = ] 0 MavlinkFTP::_workReset() -50.0% -16 [ = ] 0 MavlinkStreamAutopilotStateForGimbalDevice::send() +33% +16 [ = ] 0 MavlinkStreamBatteryInfo::send() -50.0% -16 [ = ] 0 MavlinkULog::start_ack_received() +33% +16 [ = ] 0 MavlinkULog::~MavlinkULog() +50% +16 [ = ] 0 McAutotuneAttitudeControl::updateParamsImpl() -0.1% -16 [ = ] 0 [section .symtab] -3.2% -224 [ = ] 0 [Unmapped] +0.0% +628 +0.0% +224 TOTAL Updated: 2026-03-13T17:57:40 |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security fixes backported from main for inclusion in v1.17.0-rc2.
This is the first time we're accepting a PR into a release branch this late. These are all security-only changes with no new features.
Fixes included:
Not included:
Draft because we're still waiting on PR #26740 (CMakeLists cleanup for removed mavlink_tests/ directory) to merge to main before cherry-picking here.