From 856d100ca8a3894fcb0545ee92835353fc2089f7 Mon Sep 17 00:00:00 2001
From: Adam Cozzette <acozzette@google.com>
Date: Thu, 10 Oct 2024 11:24:18 -0700
Subject: [PATCH] Update `VarintParse()` to try to detect accidental misuse

PiperOrigin-RevId: 684513423
---
 src/google/protobuf/descriptor_database.cc |  2 ++
 src/google/protobuf/parse_context.h        |  9 ++++++---
 src/google/protobuf/port.h                 | 12 ++++++++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/google/protobuf/descriptor_database.cc b/src/google/protobuf/descriptor_database.cc
index 10ebeff62d..733c9210c6 100644
--- a/src/google/protobuf/descriptor_database.cc
+++ b/src/google/protobuf/descriptor_database.cc
@@ -12,6 +12,7 @@
 #include "google/protobuf/descriptor_database.h"
 
 #include <algorithm>
+#include <cstdint>
 #include <string>
 #include <utility>
 #include <vector>
@@ -22,6 +23,7 @@
 #include "absl/strings/str_replace.h"
 #include "absl/strings/string_view.h"
 #include "google/protobuf/descriptor.pb.h"
+#include "google/protobuf/parse_context.h"
 
 
 namespace google {
diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h
index 22ed8212df..0ed3ed0cd7 100644
--- a/src/google/protobuf/parse_context.h
+++ b/src/google/protobuf/parse_context.h
@@ -866,6 +866,7 @@ static const char* VarintParseSlowArm(const char* p, uint64_t* out,
 // The caller must ensure that p points to at least 10 valid bytes.
 template <typename T>
 PROTOBUF_NODISCARD const char* VarintParse(const char* p, T* out) {
+  AssertBytesAreReadable(p, 10);
 #if defined(__aarch64__) && defined(ABSL_IS_LITTLE_ENDIAN) && !defined(_MSC_VER)
   // This optimization is not supported in big endian mode
   uint64_t first8;
@@ -1044,9 +1045,11 @@ inline const char* ParseBigVarint(const char* p, uint64_t* out) {
 
 PROTOBUF_EXPORT
 std::pair<const char*, int32_t> ReadSizeFallback(const char* p, uint32_t first);
-// Used for tags, could read up to 5 bytes which must be available. Additionally
-// it makes sure the unsigned value fits a int32_t, otherwise returns nullptr.
-// Caller must ensure its safe to call.
+
+// Used for length prefixes. Could read up to 5 bytes, but no more than
+// necessary for a single varint. The caller must ensure enough bytes are
+// available. Additionally it makes sure the unsigned value fits in an int32_t,
+// otherwise returns nullptr. Caller must ensure it is safe to call.
 inline uint32_t ReadSize(const char** pp) {
   auto p = *pp;
   uint32_t res = static_cast<uint8_t>(p[0]);
diff --git a/src/google/protobuf/port.h b/src/google/protobuf/port.h
index 3a0162ca94..6e3ebb955d 100644
--- a/src/google/protobuf/port.h
+++ b/src/google/protobuf/port.h
@@ -293,6 +293,18 @@ constexpr bool DebugHardenFuzzMessageSpaceUsedLong() {
   return false;
 }
 
+// Reads n bytes from p, if PerformDebugChecks() is true. This allows ASAN to
+// detect if a range of memory is not valid when we expect it to be. The
+// volatile keyword is necessary here to prevent the compiler from optimizing
+// away the memory reads below.
+inline void AssertBytesAreReadable(const volatile char* p, int n) {
+  if (PerformDebugChecks()) {
+    for (int i = 0; i < n; ++i) {
+      p[i];
+    }
+  }
+}
+
 // Returns true if pointers are 8B aligned, leaving least significant 3 bits
 // available.
 inline constexpr bool PtrIsAtLeast8BAligned() { return alignof(void*) >= 8; }