fix(mavlink): drop FTP boot-hook deny, keep symlink hardening

The previous commits in this branch blocked all writes under
PX4_STORAGEDIR/etc/ as a defence against attacker-controlled boot
hook files (rc.txt, config.txt, extras.txt). That subtree is the
documented user customization point for PX4 startup, and QGC's
MAVFTP file manager is the supported way to upload it. Blocking
writes there breaks a legitimate workflow on top of an already
unauthenticated channel where the real mitigation is MAVLink
signing.

Drop the boot-hook deny block. The symlink/canonicalization
hardening (\_validatePathIsInRoot, O_NOFOLLOW, parent-resolved
realpath fallback) for GHSA-93v7-287q-qx4g stays.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>
This commit is contained in:
Ramon Roche
2026-04-07 18:38:14 -07:00
parent 98c8447ef3
commit 446a66149a
+1 -48
View File
@@ -1261,52 +1261,5 @@ bool MavlinkFTP::_validatePathIsInRoot(const char *path)
bool MavlinkFTP::_validatePathIsWritable(const char *path)
{
if (!_validatePathIsInRoot(path)) {
return false;
}
// Reject writes to boot-executed startup hook files. The PX4 NuttX rcS
// sources $FRC, $FCONFIG, $FEXTRAS unconditionally during boot, so an
// attacker that can write to these paths gets persistent code execution
// at the next reboot. Reject the entire PX4_STORAGEDIR/etc/ subtree to
// keep future hooks safe by default.
#ifndef __PX4_NUTTX
// On POSIX, run the deny check on the same canonical path used by the
// in-root check (including the parent+leaf fallback for new files), so
// that prefix variations like ".//etc/x" or "./etc/x" cannot bypass the
// raw string match, and so symlinks redirecting through etc/ are caught.
char canonical_root[PATH_MAX];
char canonical[PATH_MAX];
if (realpath(PX4_STORAGEDIR, canonical_root) == nullptr
|| !canonicalize_path(path, canonical, sizeof(canonical))) {
PX4_ERR("FTP: cannot canonicalize for boot-hook check: %s", path);
return false;
}
char boot_hook_prefix[PATH_MAX];
const int n = snprintf(boot_hook_prefix, sizeof(boot_hook_prefix),
"%s/etc/", canonical_root);
if (n <= 0 || (size_t)n >= sizeof(boot_hook_prefix)) {
return false;
}
if (strncmp(canonical, boot_hook_prefix, strlen(boot_hook_prefix)) == 0) {
PX4_ERR("FTP: refusing to write protected path %s -> %s", path, canonical);
return false;
}
#else
// NuttX: simple literal prefix check.
static const char kBootHookPrefix[] = PX4_STORAGEDIR "/etc/";
if (strncmp(path, kBootHookPrefix, sizeof(kBootHookPrefix) - 1) == 0) {
PX4_ERR("FTP: refusing to write protected path %s", path);
return false;
}
#endif
return true;
return _validatePathIsInRoot(path);
}