diff --git a/src/time_zone_info.cc b/src/time_zone_info.cc index 9ccb6a4..8820762 100644 --- a/src/time_zone_info.cc +++ b/src/time_zone_info.cc @@ -421,18 +421,25 @@ inline FilePtr FOpen(const char* path) { #endif } +// Returns true if c separates path components. Windows accepts either +// form, so a "..\" component walks up a directory just like a "../" one. +inline bool IsPathSeparator(char c) { +#if defined(_WIN32) + return c == '/' || c == '\\'; +#else + return c == '/'; +#endif +} + // Returns true if the zone name starting at pos contains an unsafe path. -inline bool UnsafePath(const std::string& name, std::size_t pos) { - // Path traversal: exact match ".." - if (name.compare(pos, std::string::npos, "..") == 0) return true; - // Path traversal: leading component "../" - if (name.compare(pos, 3, "../") == 0) return true; - // Path traversal: interior component "/../" - if (name.find("/../", pos) != std::string::npos) return true; - // Path traversal: trailing component "/.." - if (name.size() - pos >= 3 && - name.compare(name.size() - 3, 3, "/..") == 0) { - return true; +bool UnsafePath(const std::string& name, std::size_t pos) { + // Path traversal: a ".." component that is at the beginning or preceded + // by a separator, and at the end or followed by a separator. + for (auto i = pos; (i = name.find("..", i)) != std::string::npos; i += 2) { + if ((i == pos || IsPathSeparator(name[i - 1])) && + (i == name.size() - 2 || IsPathSeparator(name[i + 2]))) { + return true; + } } return false; } diff --git a/src/time_zone_lookup_test.cc b/src/time_zone_lookup_test.cc index e2286a0..a386879 100644 --- a/src/time_zone_lookup_test.cc +++ b/src/time_zone_lookup_test.cc @@ -180,12 +180,6 @@ TEST(TimeZone, Failures) { EXPECT_EQ(chrono::system_clock::from_time_t(0), convert(civil_second(1970, 1, 1, 0, 0, 0), tz)); // UTC - // Reject path-traversal components. - EXPECT_FALSE(load_time_zone("file:../etc/passwd", &tz)); - EXPECT_FALSE(load_time_zone("file:../../etc/passwd", &tz)); - EXPECT_FALSE(load_time_zone("file:/../etc/passwd", &tz)); - EXPECT_FALSE(load_time_zone("file:America/../America/Los_Angeles", &tz)); - // Reject a fixed-offset name with a NUL where a digit belongs. for (const int i : {10, 11, 13, 14, 16, 17}) { std::string name = "Fixed/UTC+00:00:00"; @@ -193,10 +187,28 @@ TEST(TimeZone, Failures) { EXPECT_FALSE(load_time_zone(name, &tz)) << "NUL at offset " << i; } - // Reject non-regular files and directories. + // Reject path-traversal components. + EXPECT_FALSE(load_time_zone("file:../etc/passwd", &tz)); + EXPECT_FALSE(load_time_zone("file:../../etc/passwd", &tz)); + EXPECT_FALSE(load_time_zone("file:/../etc/passwd", &tz)); + EXPECT_FALSE(load_time_zone("file:America/../America/Los_Angeles", &tz)); + +#if defined(_WIN32) + // Windows accepts '\' as a path separator, so these escape as well. + // If they were admitted, the second would resolve back into the zoneinfo + // directory and load, failing the test. Elsewhere '\' is an ordinary + // filename character, so these would only fail as nonexistent names. + EXPECT_FALSE(load_time_zone("file:..\\etc\\passwd", &tz)); + EXPECT_FALSE(load_time_zone("file:America\\..\\America/Los_Angeles", &tz)); +#endif + +#if !defined(_MSC_VER) + // Reject non-regular files and directories. The check lives in the + // non-MSVC FOpen(), so only expect it there. EXPECT_FALSE(load_time_zone("file:/dev/null", &tz)); EXPECT_FALSE(load_time_zone("file:/dev/stdin", &tz)); EXPECT_FALSE(load_time_zone("file:/tmp", &tz)); +#endif } TEST(TimeZone, Equality) {