From 229565c7910409bf9bbdf02de5563a9c6a8cfd5e Mon Sep 17 00:00:00 2001 From: Xiaoxi Wang Date: Tue, 10 Jan 2023 23:58:51 -0800 Subject: [PATCH] check SetVersionstampedKey offset --- fdbclient/include/fdbclient/Atomic.h | 19 ++++++++++--------- fdbserver/CommitProxyServer.actor.cpp | 3 ++- .../workloads/FuzzApiCorrectness.actor.cpp | 2 +- .../TenantManagementWorkload.actor.cpp | 3 +-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/fdbclient/include/fdbclient/Atomic.h b/fdbclient/include/fdbclient/Atomic.h index 61f948e38f..d39307d668 100644 --- a/fdbclient/include/fdbclient/Atomic.h +++ b/fdbclient/include/fdbclient/Atomic.h @@ -255,6 +255,13 @@ static void placeVersionstamp(uint8_t* destination, Version version, uint16_t tr memcpy(destination + sizeof(version), &transactionNumber, sizeof(transactionNumber)); } +inline int32_t parseGetVersionStampKetOffset(StringRef& key) { + ASSERT_GE(key.size(), 4); + int32_t pos; + memcpy(&pos, key.end() - sizeof(int32_t), sizeof(int32_t)); + pos = littleEndian32(pos); + return pos; +} /* * Returns the range corresponding to the specified versionstamp key. */ @@ -265,9 +272,7 @@ inline KeyRangeRef getVersionstampKeyRange(Arena& arena, const KeyRef& key, Vers if (begin.size() < 4) throw client_invalid_operation(); - int32_t pos; - memcpy(&pos, begin.end() - sizeof(int32_t), sizeof(int32_t)); - pos = littleEndian32(pos); + auto pos = parseGetVersionStampKetOffset(begin); begin = begin.substr(0, begin.size() - 4); end = end.substr(0, end.size() - 3); mutateString(end)[end.size() - 1] = 0; @@ -285,9 +290,7 @@ inline void transformVersionstampKey(StringRef& key, Version version, uint16_t t if (key.size() < 4) throw client_invalid_operation(); - int32_t pos; - memcpy(&pos, key.end() - sizeof(int32_t), sizeof(int32_t)); - pos = littleEndian32(pos); + auto pos = parseGetVersionStampKetOffset(key); if (pos < 0 || pos + 10 > key.size()) throw client_invalid_operation(); @@ -299,9 +302,7 @@ inline void transformVersionstampMutation(MutationRef& mutation, Version version, uint16_t transactionNumber) { if ((mutation.*param).size() >= 4) { - int32_t pos; - memcpy(&pos, (mutation.*param).end() - sizeof(int32_t), sizeof(int32_t)); - pos = littleEndian32(pos); + auto pos = parseGetVersionStampKetOffset(mutation.*param); mutation.*param = (mutation.*param).substr(0, (mutation.*param).size() - 4); if (pos >= 0 && pos + 10 <= (mutation.*param).size()) { diff --git a/fdbserver/CommitProxyServer.actor.cpp b/fdbserver/CommitProxyServer.actor.cpp index 2ec7a0d5ed..b2ad665212 100644 --- a/fdbserver/CommitProxyServer.actor.cpp +++ b/fdbserver/CommitProxyServer.actor.cpp @@ -1168,7 +1168,8 @@ int64_t extractTenantIdFromSingleKeyMutation(MutationRef m) { ASSERT(!isSystemKey(m.param1)); // The first 8 bytes of the key of this OP is also an 8-byte number - if (m.type == MutationRef::SetVersionstampedKey) { + if (m.type == MutationRef::SetVersionstampedKey && m.param1.size() >= 4 && + parseGetVersionStampKetOffset(m.param1) < 8) { return TenantInfo::INVALID_TENANT; } diff --git a/fdbserver/workloads/FuzzApiCorrectness.actor.cpp b/fdbserver/workloads/FuzzApiCorrectness.actor.cpp index 5ec5d696b0..093f21bc66 100644 --- a/fdbserver/workloads/FuzzApiCorrectness.actor.cpp +++ b/fdbserver/workloads/FuzzApiCorrectness.actor.cpp @@ -272,7 +272,7 @@ struct FuzzApiCorrectnessWorkload : TestWorkload { } Future check(Database const& cx) override { - if (useSystemKeys) { // there must be illegal access during data load + if (writeSystemKeys) { // there must be illegal access during data load return illegalTenantAccess; } return success; diff --git a/fdbserver/workloads/TenantManagementWorkload.actor.cpp b/fdbserver/workloads/TenantManagementWorkload.actor.cpp index 63ca878d0f..86fe3c5c4b 100644 --- a/fdbserver/workloads/TenantManagementWorkload.actor.cpp +++ b/fdbserver/workloads/TenantManagementWorkload.actor.cpp @@ -68,8 +68,7 @@ struct TenantManagementWorkload : TestWorkload { int64_t maxId = -1; const Key keyName = "key"_sr; - const Key testParametersKey = - "test_parameters"_sr; // nonMetadataSystemKeys.begin.withSuffix("/tenant_test/test_parameters"_sr); + const Key testParametersKey = nonMetadataSystemKeys.begin.withSuffix("/tenant_test/test_parameters"_sr); const Value noTenantValue = "no_tenant"_sr; const TenantName tenantNamePrefix = "tenant_management_workload_"_sr; const ClusterName dataClusterName = "cluster1"_sr;