From 4e9d9d989f937b40472e4fae88b49fd99d1988ba Mon Sep 17 00:00:00 2001 From: Kosie van der Merwe Date: Wed, 9 Jan 2013 10:44:30 -0800 Subject: [PATCH] Fixed wrong assumption in Table::Open() Summary: `Table::Open()` assumes that `size` correctly describes the size of `file`, added a check that the footer is actually the right size and for good measure added assertions to `Footer::DecodeFrom()`. This was discovered by running `valgrind ./db_test` and seeing that `Footer::DecodeFrom()` was accessing uninitialized memory. Test Plan: make clean check ran `valgrind ./db_test` and saw DBTest.NoSpace no longer complains about a conditional jump being dependent on uninitialized memory. Reviewers: dhruba, vamsi, emayanke, sheki Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D7815 --- table/format.cc | 3 +++ table/table.cc | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/table/format.cc b/table/format.cc index baa48db4bb..d847d40929 100644 --- a/table/format.cc +++ b/table/format.cc @@ -42,6 +42,9 @@ void Footer::EncodeTo(std::string* dst) const { } Status Footer::DecodeFrom(Slice* input) { + assert(input != NULL); + assert(input->size() >= kEncodedLength); + const char* magic_ptr = input->data() + kEncodedLength - 8; const uint32_t magic_lo = DecodeFixed32(magic_ptr); const uint32_t magic_hi = DecodeFixed32(magic_ptr + 4); diff --git a/table/table.cc b/table/table.cc index 062bf7abd9..394f65e7d9 100644 --- a/table/table.cc +++ b/table/table.cc @@ -51,6 +51,13 @@ Status Table::Open(const Options& options, &footer_input, footer_space); if (!s.ok()) return s; + // Check that we actually read the whole footer from the file. It may be + // that size isn't correct. + if (footer_input.size() != Footer::kEncodedLength) { + return Status::InvalidArgument("file is too short to be an sstable"); + } + + Footer footer; s = footer.DecodeFrom(&footer_input); if (!s.ok()) return s;