diff --git a/src/xmodem.cpp b/src/xmodem.cpp index ce7ad8020d0..58339a369dc 100644 --- a/src/xmodem.cpp +++ b/src/xmodem.cpp @@ -62,10 +62,13 @@ bool XModemAdapter::isValidFilename(const char *name) { if (!name || name[0] == '\0') return false; + const bool driveLetter = (name[0] >= 'A' && name[0] <= 'Z') || (name[0] >= 'a' && name[0] <= 'z'); + if (driveLetter && name[1] == ':') + return false; // Reject any ".." path component. Absolute paths and subdirectories are fine; they stay within // the filesystem root, so only traversal out of it needs blocking. for (const char *seg = name; *seg;) { - const char *slash = strchr(seg, '/'); + const char *slash = strpbrk(seg, "/\\"); const size_t len = slash ? (size_t)(slash - seg) : strlen(seg); if (len == 2 && seg[0] == '.' && seg[1] == '.') return false; diff --git a/test/test_xmodem/test_main.cpp b/test/test_xmodem/test_main.cpp index 816c78e9c01..c6a20fdf051 100644 --- a/test/test_xmodem/test_main.cpp +++ b/test/test_xmodem/test_main.cpp @@ -21,6 +21,22 @@ void test_xmodem_rejects_dotdot_traversal(void) TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("/..")); } +void test_xmodem_rejects_backslash_traversal(void) +{ + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("..\\secret")); + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("..\\..\\Windows\\System32\\drivers\\etc\\hosts")); + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("dir\\..\\..\\x")); + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("dir/..\\x")); + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("dir\\..")); +} + +void test_xmodem_rejects_drive_qualified(void) +{ + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("C:\\Windows\\System32\\x")); + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("C:/Windows/System32/x")); + TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("c:relative.txt")); +} + void test_xmodem_rejects_empty(void) { TEST_ASSERT_FALSE(XModemAdapter::isValidFilename("")); @@ -36,6 +52,9 @@ void test_xmodem_allows_legit_paths(void) // ".." only inside a name (not a whole component) is a valid filename, not traversal. TEST_ASSERT_TRUE(XModemAdapter::isValidFilename("my..file")); TEST_ASSERT_TRUE(XModemAdapter::isValidFilename("...")); + // A colon that cannot form a drive qualifier is a legal filename on the POSIX daemon. + TEST_ASSERT_TRUE(XModemAdapter::isValidFilename("1:30pm.txt")); + TEST_ASSERT_TRUE(XModemAdapter::isValidFilename("dir/1:30pm.txt")); } #endif // FSCom @@ -46,6 +65,8 @@ void setup() UNITY_BEGIN(); #ifdef FSCom RUN_TEST(test_xmodem_rejects_dotdot_traversal); + RUN_TEST(test_xmodem_rejects_backslash_traversal); + RUN_TEST(test_xmodem_rejects_drive_qualified); RUN_TEST(test_xmodem_rejects_empty); RUN_TEST(test_xmodem_allows_legit_paths); #endif