From a79c7c05e8f3e5b1051ec2a32b9a7220b6e41cc8 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Fri, 15 Dec 2017 12:22:13 -0800 Subject: [PATCH] fix backup meta-file buffer overrun Summary: - check most times after calling snprintf that the buffer didn't fill up. Previously we'd proceed and use `buf_size - len` as the length in subsequent calls, which underflowed as those are unsigned size_t. - replace some memcpys with snprintf for consistency Closes https://github.com/facebook/rocksdb/pull/3255 Differential Revision: D6541464 Pulled By: ajkr fbshipit-source-id: 8610ea6a24f38e0a37c6d17bc65b7c712da6d932 --- utilities/backupable/backupable_db.cc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/utilities/backupable/backupable_db.cc b/utilities/backupable/backupable_db.cc index d8d331044..e484eea36 100644 --- a/utilities/backupable/backupable_db.cc +++ b/utilities/backupable/backupable_db.cc @@ -1722,23 +1722,25 @@ Status BackupEngineImpl::BackupMeta::StoreToFile(bool sync) { if (!app_metadata_.empty()) { std::string hex_encoded_metadata = Slice(app_metadata_).ToString(/* hex */ true); - if (hex_encoded_metadata.size() + kMetaDataPrefix.size() + 1 > - buf_size - len) { + len += snprintf(buf.get() + len, buf_size - len, "%s%s\n", + kMetaDataPrefix.ToString().c_str(), + hex_encoded_metadata.c_str()); + if (len >= buf_size) { return Status::Corruption("Buffer too small to fit backup metadata"); } - memcpy(buf.get() + len, kMetaDataPrefix.data(), kMetaDataPrefix.size()); - len += kMetaDataPrefix.size(); - memcpy(buf.get() + len, hex_encoded_metadata.data(), - hex_encoded_metadata.size()); - len += hex_encoded_metadata.size(); - buf[len++] = '\n'; } len += snprintf(buf.get() + len, buf_size - len, "%" ROCKSDB_PRIszt "\n", files_.size()); + if (len >= buf_size) { + return Status::Corruption("Buffer too small to fit backup metadata"); + } for (const auto& file : files_) { // use crc32 for now, switch to something else if needed len += snprintf(buf.get() + len, buf_size - len, "%s crc32 %u\n", file->filename.c_str(), file->checksum_value); + if (len >= buf_size) { + return Status::Corruption("Buffer too small to fit backup metadata"); + } } s = backup_meta_file->Append(Slice(buf.get(), len));