mirror of
https://gitee.com/mirrors_PX4/PX4-Autopilot.git
synced 2026-10-31 06:51:35 +08:00
fix(mavlink): reject path traversal sequences in FTP operations
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>
This commit is contained in:
@@ -362,6 +362,10 @@ MavlinkFTP::_workList(PayloadHeader *payload)
|
|||||||
{
|
{
|
||||||
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
||||||
|
|
||||||
|
if (!_validatePath(_work_buffer1)) {
|
||||||
|
return kErrFailFileProtected;
|
||||||
|
}
|
||||||
|
|
||||||
ErrorCode errorCode = kErrNone;
|
ErrorCode errorCode = kErrNone;
|
||||||
unsigned offset = 0;
|
unsigned offset = 0;
|
||||||
|
|
||||||
@@ -510,6 +514,10 @@ MavlinkFTP::_workOpen(PayloadHeader *payload, int oflag)
|
|||||||
|
|
||||||
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
||||||
|
|
||||||
|
if (!_validatePath(_work_buffer1)) {
|
||||||
|
return kErrFailFileProtected;
|
||||||
|
}
|
||||||
|
|
||||||
PX4_DEBUG("FTP: open '%s'", _work_buffer1);
|
PX4_DEBUG("FTP: open '%s'", _work_buffer1);
|
||||||
|
|
||||||
uint32_t fileSize = 0;
|
uint32_t fileSize = 0;
|
||||||
@@ -652,7 +660,7 @@ MavlinkFTP::_workRemoveFile(PayloadHeader *payload)
|
|||||||
{
|
{
|
||||||
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
||||||
|
|
||||||
if (!_validatePathIsWritable(_work_buffer1)) {
|
if (!_validatePath(_work_buffer1) || !_validatePathIsWritable(_work_buffer1)) {
|
||||||
return kErrFailFileProtected;
|
return kErrFailFileProtected;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -676,7 +684,7 @@ MavlinkFTP::_workTruncateFile(PayloadHeader *payload)
|
|||||||
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
||||||
payload->size = 0;
|
payload->size = 0;
|
||||||
|
|
||||||
if (!_validatePathIsWritable(_work_buffer1)) {
|
if (!_validatePath(_work_buffer1) || !_validatePathIsWritable(_work_buffer1)) {
|
||||||
return kErrFailFileProtected;
|
return kErrFailFileProtected;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -837,7 +845,7 @@ MavlinkFTP::_workRename(PayloadHeader *payload)
|
|||||||
_constructPath(_work_buffer1, _work_buffer1_len, ptr);
|
_constructPath(_work_buffer1, _work_buffer1_len, ptr);
|
||||||
_constructPath(_work_buffer2, _work_buffer2_len, ptr + oldpath_sz + 1);
|
_constructPath(_work_buffer2, _work_buffer2_len, ptr + oldpath_sz + 1);
|
||||||
|
|
||||||
if (!_validatePathIsWritable(_work_buffer2)) {
|
if (!_validatePath(_work_buffer1) || !_validatePath(_work_buffer2) || !_validatePathIsWritable(_work_buffer2)) {
|
||||||
return kErrFailFileProtected;
|
return kErrFailFileProtected;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -860,7 +868,7 @@ MavlinkFTP::_workRemoveDirectory(PayloadHeader *payload)
|
|||||||
{
|
{
|
||||||
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
||||||
|
|
||||||
if (!_validatePathIsWritable(_work_buffer1)) {
|
if (!_validatePath(_work_buffer1) || !_validatePathIsWritable(_work_buffer1)) {
|
||||||
return kErrFailFileProtected;
|
return kErrFailFileProtected;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -883,7 +891,7 @@ MavlinkFTP::_workCreateDirectory(PayloadHeader *payload)
|
|||||||
{
|
{
|
||||||
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer1, _work_buffer1_len, _data_as_cstring(payload));
|
||||||
|
|
||||||
if (!_validatePathIsWritable(_work_buffer1)) {
|
if (!_validatePath(_work_buffer1) || !_validatePathIsWritable(_work_buffer1)) {
|
||||||
return kErrFailFileProtected;
|
return kErrFailFileProtected;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -908,6 +916,10 @@ MavlinkFTP::_workCalcFileCRC32(PayloadHeader *payload)
|
|||||||
ssize_t bytes_read;
|
ssize_t bytes_read;
|
||||||
_constructPath(_work_buffer2, _work_buffer2_len, _data_as_cstring(payload));
|
_constructPath(_work_buffer2, _work_buffer2_len, _data_as_cstring(payload));
|
||||||
|
|
||||||
|
if (!_validatePath(_work_buffer2)) {
|
||||||
|
return kErrFailFileProtected;
|
||||||
|
}
|
||||||
|
|
||||||
int fd = ::open(_work_buffer2, O_RDONLY);
|
int fd = ::open(_work_buffer2, O_RDONLY);
|
||||||
|
|
||||||
if (fd < 0) {
|
if (fd < 0) {
|
||||||
@@ -1147,6 +1159,36 @@ void MavlinkFTP::send()
|
|||||||
} while (more_data);
|
} while (more_data);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reject paths containing ".." components to prevent directory traversal
|
||||||
|
* outside of _root_dir. Walks the path checking each component between
|
||||||
|
* '/' separators. A component of exactly ".." (followed by '/' or end
|
||||||
|
* of string) is rejected.
|
||||||
|
*
|
||||||
|
* Examples:
|
||||||
|
* "/fs/microsd/logs" -> allowed
|
||||||
|
* "/fs/microsd/../etc" -> rejected
|
||||||
|
* "../../tmp/pwned" -> rejected
|
||||||
|
* "/fs/microsd/logs/.." -> rejected
|
||||||
|
* "/fs/microsd/..hidden" -> allowed (not a ".." component)
|
||||||
|
*/
|
||||||
|
bool MavlinkFTP::_validatePath(const char *path)
|
||||||
|
{
|
||||||
|
const char *p = path;
|
||||||
|
|
||||||
|
while (*p != '\0') {
|
||||||
|
if ((p == path || *(p - 1) == '/') && p[0] == '.' && p[1] == '.'
|
||||||
|
&& (p[2] == '/' || p[2] == '\0')) {
|
||||||
|
PX4_ERR("FTP: rejecting path traversal in %s", path);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
p++;
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
bool MavlinkFTP::_validatePathIsWritable(const char *path)
|
bool MavlinkFTP::_validatePathIsWritable(const char *path)
|
||||||
{
|
{
|
||||||
#ifdef __PX4_NUTTX
|
#ifdef __PX4_NUTTX
|
||||||
|
|||||||
@@ -156,6 +156,7 @@ private:
|
|||||||
*/
|
*/
|
||||||
void _constructPath(char *dst, int dst_len, const char *path) const;
|
void _constructPath(char *dst, int dst_len, const char *path) const;
|
||||||
|
|
||||||
|
bool _validatePath(const char *path);
|
||||||
bool _validatePathIsWritable(const char *path);
|
bool _validatePathIsWritable(const char *path);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -200,7 +201,7 @@ private:
|
|||||||
hrt_abstime _last_work_buffer_access{0}; ///< timestamp when the buffers were last accessed
|
hrt_abstime _last_work_buffer_access{0}; ///< timestamp when the buffers were last accessed
|
||||||
|
|
||||||
// prepend a root directory to each file/dir access to avoid enumerating the full FS tree (e.g. on Linux).
|
// prepend a root directory to each file/dir access to avoid enumerating the full FS tree (e.g. on Linux).
|
||||||
// Note that requests can still fall outside of the root dir by using ../..
|
// Path traversal via ".." is rejected by _validatePath().
|
||||||
#ifdef MAVLINK_FTP_UNIT_TEST
|
#ifdef MAVLINK_FTP_UNIT_TEST
|
||||||
static constexpr const char _root_dir[] = "";
|
static constexpr const char _root_dir[] = "";
|
||||||
#else
|
#else
|
||||||
|
|||||||
Reference in New Issue
Block a user