Handling misuse of snprintf return value (#7686)

Summary:
Handle misuse of snprintf return value to avoid Out of bound
read/write.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7686

Test Plan: make check -j64

Reviewed By: riversand963

Differential Revision: D25030831

Pulled By: akankshamahajan15

fbshipit-source-id: 1a1d181c067c78b94d720323ae00b79566b57cfa
main
Akanksha Mahajan 4 years ago committed by Facebook GitHub Bot
parent b77569f18b
commit 20c7d7c58a
  1. 2
      db/internal_stats.cc
  2. 63
      utilities/backupable/backupable_db.cc
  3. 29
      utilities/blob_db/blob_db_impl.cc

@ -61,6 +61,7 @@ void PrintLevelStatsHeader(char* buf, size_t len, const std::string& cf_name,
const std::string& group_by) {
int written_size =
snprintf(buf, len, "\n** Compaction Stats [%s] **\n", cf_name.c_str());
written_size = std::min(written_size, static_cast<int>(len));
auto hdr = [](LevelStatType t) {
return InternalStats::compaction_level_stats.at(t).header_name.c_str();
};
@ -80,6 +81,7 @@ void PrintLevelStatsHeader(char* buf, size_t len, const std::string& cf_name,
hdr(LevelStatType::KEY_DROP));
written_size += line_size;
written_size = std::min(written_size, static_cast<int>(len));
snprintf(buf + written_size, len - written_size, "%s\n",
std::string(line_size, '-').c_str());
}

@ -2245,70 +2245,25 @@ Status BackupEngineImpl::BackupMeta::StoreToFile(bool sync) {
return s;
}
std::unique_ptr<char[]> buf(new char[max_backup_meta_file_size_]);
size_t len = 0, buf_size = max_backup_meta_file_size_;
len += snprintf(buf.get(), buf_size, "%" PRId64 "\n", timestamp_);
len += snprintf(buf.get() + len, buf_size - len, "%" PRIu64 "\n",
sequence_number_);
std::ostringstream buf;
buf << timestamp_ << "\n";
buf << sequence_number_ << "\n";
if (!app_metadata_.empty()) {
std::string hex_encoded_metadata =
Slice(app_metadata_).ToString(/* hex */ true);
// +1 to accommodate newline character
size_t hex_meta_strlen =
kMetaDataPrefix.ToString().length() + hex_encoded_metadata.length() + 1;
if (hex_meta_strlen >= buf_size) {
return Status::Corruption("Buffer too small to fit backup metadata");
}
else if (len + hex_meta_strlen >= buf_size) {
backup_meta_file->Append(Slice(buf.get(), len));
buf.reset();
std::unique_ptr<char[]> new_reset_buf(
new char[max_backup_meta_file_size_]);
buf.swap(new_reset_buf);
len = 0;
}
len += snprintf(buf.get() + len, buf_size - len, "%s%s\n",
kMetaDataPrefix.ToString().c_str(),
hex_encoded_metadata.c_str());
}
char writelen_temp[19];
if (len + snprintf(writelen_temp, sizeof(writelen_temp),
"%" ROCKSDB_PRIszt "\n", files_.size()) >= buf_size) {
backup_meta_file->Append(Slice(buf.get(), len));
buf.reset();
std::unique_ptr<char[]> new_reset_buf(new char[max_backup_meta_file_size_]);
buf.swap(new_reset_buf);
len = 0;
}
{
const char *const_write = writelen_temp;
len += snprintf(buf.get() + len, buf_size - len, "%s", const_write);
buf << kMetaDataPrefix.ToString() << hex_encoded_metadata << "\n";
}
buf << files_.size() << "\n";
for (const auto& file : files_) {
// use crc32c for now, switch to something else if needed
// WART: The checksums are crc32c, not original crc32
size_t newlen =
len + file->filename.length() +
snprintf(writelen_temp, sizeof(writelen_temp), " crc32 %u\n",
ChecksumHexToInt32(file->checksum_hex));
const char* const_write = writelen_temp;
if (newlen >= buf_size) {
backup_meta_file->Append(Slice(buf.get(), len));
buf.reset();
std::unique_ptr<char[]> new_reset_buf(
new char[max_backup_meta_file_size_]);
buf.swap(new_reset_buf);
len = 0;
}
len += snprintf(buf.get() + len, buf_size - len, "%s%s",
file->filename.c_str(), const_write);
buf << file->filename << " crc32 " << ChecksumHexToInt32(file->checksum_hex)
<< "\n";
}
s = backup_meta_file->Append(Slice(buf.get(), len));
s = backup_meta_file->Append(Slice(buf.str()));
if (s.ok() && sync) {
s = backup_meta_file->Sync();
}

@ -1686,35 +1686,30 @@ std::pair<bool, int64_t> BlobDBImpl::SanityCheck(bool aborted) {
for (auto blob_file_pair : blob_files_) {
auto blob_file = blob_file_pair.second;
char buf[1000];
int pos = snprintf(buf, sizeof(buf),
"Blob file %" PRIu64 ", size %" PRIu64
", blob count %" PRIu64 ", immutable %d",
blob_file->BlobFileNumber(), blob_file->GetFileSize(),
blob_file->BlobCount(), blob_file->Immutable());
std::ostringstream buf;
buf << "Blob file " << blob_file->BlobFileNumber() << ", size "
<< blob_file->GetFileSize() << ", blob count " << blob_file->BlobCount()
<< ", immutable " << blob_file->Immutable();
if (blob_file->HasTTL()) {
ExpirationRange expiration_range;
{
ReadLock file_lock(&blob_file->mutex_);
expiration_range = blob_file->GetExpirationRange();
}
buf << ", expiration range (" << expiration_range.first << ", "
<< expiration_range.second << ")";
pos += snprintf(buf + pos, sizeof(buf) - pos,
", expiration range (%" PRIu64 ", %" PRIu64 ")",
expiration_range.first, expiration_range.second);
if (!blob_file->Obsolete()) {
pos += snprintf(buf + pos, sizeof(buf) - pos,
", expire in %" PRIu64 " seconds",
expiration_range.second - now);
buf << ", expire in " << (expiration_range.second - now) << "seconds";
}
}
if (blob_file->Obsolete()) {
pos += snprintf(buf + pos, sizeof(buf) - pos, ", obsolete at %" PRIu64,
blob_file->GetObsoleteSequence());
buf << ", obsolete at " << blob_file->GetObsoleteSequence();
}
snprintf(buf + pos, sizeof(buf) - pos, ".");
ROCKS_LOG_INFO(db_options_.info_log, "%s", buf);
buf << ".";
ROCKS_LOG_INFO(db_options_.info_log, "%s", buf.str().c_str());
}
// reschedule

Loading…
Cancel
Save