diff --git a/src/modules/mavlink/mavlink_ftp.cpp b/src/modules/mavlink/mavlink_ftp.cpp index 0ed2649f4d..32eaf9624d 100644 --- a/src/modules/mavlink/mavlink_ftp.cpp +++ b/src/modules/mavlink/mavlink_ftp.cpp @@ -616,6 +616,10 @@ MavlinkFTP::_workWrite(PayloadHeader *payload) return kErrInvalidSession; } + if (!_validatePathIsWritable(_work_buffer1)) { + return kErrFailFileProtected; + } + if (lseek(_session_info.fd, payload->offset, SEEK_SET) < 0) { // Unable to see to the specified location PX4_ERR("seek fail"); @@ -647,6 +651,10 @@ MavlinkFTP::_workRemoveFile(PayloadHeader *payload) // ensure termination _work_buffer1[_work_buffer1_len - 1] = '\0'; + if (!_validatePathIsWritable(_work_buffer1)) { + return kErrFailFileProtected; + } + PX4_DEBUG("unlink %s", _work_buffer1); if (unlink(_work_buffer1) == 0) { @@ -670,6 +678,10 @@ MavlinkFTP::_workTruncateFile(PayloadHeader *payload) _work_buffer1[_work_buffer1_len - 1] = '\0'; payload->size = 0; + if (!_validatePathIsWritable(_work_buffer1)) { + return kErrFailFileProtected; + } + #ifdef __PX4_NUTTX // emulate truncate(_work_buffer1, payload->offset) by @@ -832,6 +844,10 @@ MavlinkFTP::_workRename(PayloadHeader *payload) strncpy(_work_buffer2 + _root_dir_len, ptr + oldpath_sz + 1, _work_buffer2_len - _root_dir_len); _work_buffer2[_work_buffer2_len - 1] = '\0'; // ensure termination + if (!_validatePathIsWritable(_work_buffer2)) { + return kErrFailFileProtected; + } + PX4_DEBUG("rename from %s to %s", _work_buffer1, _work_buffer2); if (rename(_work_buffer1, _work_buffer2) == 0) { @@ -854,6 +870,10 @@ MavlinkFTP::_workRemoveDirectory(PayloadHeader *payload) // ensure termination _work_buffer1[_work_buffer1_len - 1] = '\0'; + if (!_validatePathIsWritable(_work_buffer1)) { + return kErrFailFileProtected; + } + PX4_DEBUG("remove dir %s", _work_buffer1); if (rmdir(_work_buffer1) == 0) { @@ -876,6 +896,10 @@ MavlinkFTP::_workCreateDirectory(PayloadHeader *payload) // ensure termination _work_buffer1[_work_buffer1_len - 1] = '\0'; + if (!_validatePathIsWritable(_work_buffer1)) { + return kErrFailFileProtected; + } + PX4_DEBUG("create dir %s", _work_buffer1); if (mkdir(_work_buffer1, S_IRWXU | S_IRWXG | S_IRWXO) == 0) { @@ -1138,3 +1162,19 @@ void MavlinkFTP::send() _reply(&ftp_msg); } while (more_data); } + +bool MavlinkFTP::_validatePathIsWritable(const char *path) +{ +#ifdef __PX4_NUTTX + + // Don't allow writes to system paths as they are in RAM + // Ideally we'd canonicalize the path (with 'realpath'), but it might not exist, so realpath() would fail. + // The next simpler thing is to check there's no reference to a parent dir. + if (strncmp(path, "/fs/microsd/", 12) != 0 || strstr(path, "/../") != nullptr) { + PX4_ERR("Disallowing write to %s", path); + return false; + } + +#endif + return true; +} diff --git a/src/modules/mavlink/mavlink_ftp.h b/src/modules/mavlink/mavlink_ftp.h index 3a92690278..4372033043 100644 --- a/src/modules/mavlink/mavlink_ftp.h +++ b/src/modules/mavlink/mavlink_ftp.h @@ -155,6 +155,8 @@ private: uint8_t _getServerComponentId(void); uint8_t _getServerChannel(void); + bool _validatePathIsWritable(const char *path); + /** * make sure that the working buffers _work_buffer* are allocated * @return true if buffers exist, false if allocation failed