Skip to content

Commit

Permalink
leveldb: Check slice length in Footer::DecodeFrom()
Browse files Browse the repository at this point in the history 
Without this check decoding the footer in Table::Open() can read
uninitialized bytes from a buffer allocated on the stack if the file
was unexpectedly short.

In practice this is probably fine since this function validates a magic
number but MSan complains about branching on uninitialized data.

PiperOrigin-RevId: 525271012
  • Loading branch information
@a-sully
leveldb Team authored and a-sully committed Apr 20, 2023
1 parent c61238d commit 068d5ee
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions table/format.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ void Footer::EncodeTo(std::string* dst) const {
}

Status Footer::DecodeFrom(Slice* input) {
if (input->size() < kEncodedLength) {
return Status::Corruption("not an sstable (footer too short)");
}

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);
Expand Down

2 comments on commit 068d5ee

@Tc69Cabusog
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all right

@DaviIcaro
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think have somes problems in this code:
Avoid Repeated Access to input->data(): In the original code, input->data() is accessed multiple times to calculate magic_ptr, magic_lo, and magic_hi. This can be optimized by accessing input->data() only once.

Minimize Duplicated Pointer Dereferencing: Optimize the code by storing the dereferenced value of a pointer in a temporary variable to avoid repetitive dereferencing.

// DecodeFrom function to decode data from a Slice
Status Footer::DecodeFrom(Slice* input) {
// Check if the input size is sufficient for decoding
if (input->size() < kEncodedLength) {
return Status::Corruption("not an sstable (footer too short)");
}

// Get a pointer to the data within the input Slice
const char* data = input->data();

// Calculate the pointer to the magic number within the data
const char* magic_ptr = data + kEncodedLength - 8;

// Decode the 64-bit magic number
uint64_t magic = DecodeFixed64(magic_ptr);

// Extract the lower 32 bits of the magic number
const uint32_t magic_lo = static_cast<uint32_t>(magic & 0xFFFFFFFFu);

// Extract the upper 32 bits of the magic number
const uint32_t magic_hi = static_cast<uint32_t>(magic >> 32);

// At this point, magic_lo and magic_hi hold the decoded magic number parts
// Further processing or actions can be performed here

// Example: return a success status
return Status::OK();
}

Please sign in to comment.