From ef465615618d0cd129fd3102d904d7151dbaa1f5 Mon Sep 17 00:00:00 2001 From: Fredrik Medley Date: Wed, 6 Sep 2017 13:23:53 +0200 Subject: Verify GetFullPathName return value GetFullPathName previously failed silently on long path names resulting in uninitialized path result. Signed-off-by: Fredrik Medley --- src/includes_normalize-win32.cc | 55 ++++++++++++++++++++++++++++++++--------- src/includes_normalize.h | 4 +-- src/includes_normalize_test.cc | 29 +++++++++++++++++++++- 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 795542b..79bf5b4 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -26,6 +26,21 @@ namespace { +bool InternalGetFullPathName(const StringPiece& file_name, char* buffer, + size_t buffer_length, string *err) { + DWORD result_size = GetFullPathNameA(file_name.AsString().c_str(), + buffer_length, buffer, NULL); + if (result_size == 0) { + *err = "GetFullPathNameA(" + file_name.AsString() + "): " + + GetLastErrorString(); + return false; + } else if (result_size > buffer_length) { + *err = "path too long"; + return false; + } + return true; +} + bool IsPathSeparator(char c) { return c == '/' || c == '\\'; } @@ -54,15 +69,19 @@ bool SameDriveFast(StringPiece a, StringPiece b) { } // Return true if paths a and b are on the same Windows drive. -bool SameDrive(StringPiece a, StringPiece b) { +bool SameDrive(StringPiece a, StringPiece b, string* err) { if (SameDriveFast(a, b)) { return true; } char a_absolute[_MAX_PATH]; char b_absolute[_MAX_PATH]; - GetFullPathNameA(a.AsString().c_str(), sizeof(a_absolute), a_absolute, NULL); - GetFullPathNameA(b.AsString().c_str(), sizeof(b_absolute), b_absolute, NULL); + if (!InternalGetFullPathName(a, a_absolute, sizeof(a_absolute), err)) { + return false; + } + if (!InternalGetFullPathName(b, b_absolute, sizeof(b_absolute), err)) { + return false; + } char a_drive[_MAX_DIR]; char b_drive[_MAX_DIR]; _splitpath(a_absolute, a_drive, NULL, NULL, NULL); @@ -106,11 +125,15 @@ bool IsFullPathName(StringPiece s) { } // anonymous namespace IncludesNormalize::IncludesNormalize(const string& relative_to) { - relative_to_ = AbsPath(relative_to); + string err; + relative_to_ = AbsPath(relative_to, &err); + if (!err.empty()) { + Fatal("Initializing IncludesNormalize(): %s", err.c_str()); + } split_relative_to_ = SplitStringPiece(relative_to_, '/'); } -string IncludesNormalize::AbsPath(StringPiece s) { +string IncludesNormalize::AbsPath(StringPiece s, string* err) { if (IsFullPathName(s)) { string result = s.AsString(); for (size_t i = 0; i < result.size(); ++i) { @@ -122,7 +145,9 @@ string IncludesNormalize::AbsPath(StringPiece s) { } char result[_MAX_PATH]; - GetFullPathNameA(s.AsString().c_str(), sizeof(result), result, NULL); + if (!InternalGetFullPathName(s, result, sizeof(result), err)) { + return ""; + } for (char* c = result; *c; ++c) if (*c == '\\') *c = '/'; @@ -130,8 +155,10 @@ string IncludesNormalize::AbsPath(StringPiece s) { } string IncludesNormalize::Relativize( - StringPiece path, const vector& start_list) { - string abs_path = AbsPath(path); + StringPiece path, const vector& start_list, string* err) { + string abs_path = AbsPath(path, err); + if (!err->empty()) + return ""; vector path_list = SplitStringPiece(abs_path, '/'); int i; for (i = 0; i < static_cast(min(start_list.size(), path_list.size())); @@ -165,12 +192,18 @@ bool IncludesNormalize::Normalize(const string& input, if (!CanonicalizePath(copy, &len, &slash_bits, err)) return false; StringPiece partially_fixed(copy, len); - string abs_input = AbsPath(partially_fixed); + string abs_input = AbsPath(partially_fixed, err); + if (!err->empty()) + return false; - if (!SameDrive(abs_input, relative_to_)) { + if (!SameDrive(abs_input, relative_to_, err)) { + if (!err->empty()) + return false; *result = partially_fixed.AsString(); return true; } - *result = Relativize(abs_input, split_relative_to_); + *result = Relativize(abs_input, split_relative_to_, err); + if (!err->empty()) + return false; return true; } diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 3811e53..0339581 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -25,9 +25,9 @@ struct IncludesNormalize { IncludesNormalize(const string& relative_to); // Internal utilities made available for testing, maybe useful otherwise. - static string AbsPath(StringPiece s); + static string AbsPath(StringPiece s, string* err); static string Relativize(StringPiece path, - const vector& start_list); + const vector& start_list, string* err); /// Normalize by fixing slashes style, fixing redundant .. and . and makes the /// path |input| relative to |this->relative_to_| and store to |result|. diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index eac36fd..dbcdbe0 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -58,9 +58,12 @@ TEST(IncludesNormalize, Simple) { } TEST(IncludesNormalize, WithRelative) { + string err; string currentdir = GetCurDir(); EXPECT_EQ("c", NormalizeRelativeAndCheckNoError("a/b/c", "a/b")); - EXPECT_EQ("a", NormalizeAndCheckNoError(IncludesNormalize::AbsPath("a"))); + EXPECT_EQ("a", + NormalizeAndCheckNoError(IncludesNormalize::AbsPath("a", &err))); + EXPECT_EQ("", err); EXPECT_EQ(string("../") + currentdir + string("/a"), NormalizeRelativeAndCheckNoError("a", "../b")); EXPECT_EQ(string("../") + currentdir + string("/a/b"), @@ -138,3 +141,27 @@ TEST(IncludesNormalize, LongInvalidPath) { EXPECT_EQ(forward_slashes.substr(cwd_len + 1), NormalizeAndCheckNoError(kExactlyMaxPath)); } + +TEST(IncludesNormalize, ShortRelativeButTooLongAbsolutePath) { + string result, err; + IncludesNormalize normalizer("."); + // A short path should work + EXPECT_TRUE(normalizer.Normalize("a", &result, &err)); + EXPECT_EQ("", err); + + // Construct max size path having cwd prefix. + // kExactlyMaxPath = "aaaa\\aaaa...aaaa\0"; + char kExactlyMaxPath[_MAX_PATH + 1]; + for (int i = 0; i < _MAX_PATH; ++i) { + if (i < _MAX_PATH - 1 && i % 10 == 4) + kExactlyMaxPath[i] = '\\'; + else + kExactlyMaxPath[i] = 'a'; + } + kExactlyMaxPath[_MAX_PATH] = '\0'; + EXPECT_EQ(strlen(kExactlyMaxPath), _MAX_PATH); + + // Make sure a path that's exactly _MAX_PATH long fails with a proper error. + EXPECT_FALSE(normalizer.Normalize(kExactlyMaxPath, &result, &err)); + EXPECT_TRUE(err.find("GetFullPathName") != string::npos); +} -- cgit v0.12