From 08c703a7b84ff42f62c2c1adc100fead84630a86 Mon Sep 17 00:00:00 2001
From: Alex Dale <sigquit@google.com>
Date: Thu, 4 Nov 2021 19:19:24 -0700
Subject: [PATCH] Added numeric value guards to ValueMetric.

[ Merge of http://go/wvgerrit/137951 ]

ValueMetric was not checking whether certain unsigned types will
overflow when casted to protobuf's signed int64 type.  This CL
adds a max value to prevent potential overflows.

The value metric has also been updated to an emum to represent its
possible three state, as opposed to using two boolean flags.  For basic
value metrics, this saves about 4 bytes of space.  This works out to
being ~100 bytes per crypto session, ~20 bytes per CDM session and ~20
bytes per engine metrics.

Bug: 204946540
Test: Metric unit tests
Change-Id: I688bece8f31cccaf2a25bd8f99d9b080b0efd0de
---
 .../cdm/metrics/include/value_metric.h        | 90 +++++++++----------
 .../cdm/metrics/src/value_metric.cpp          | 52 +++++++----
 .../metrics/test/value_metric_unittest.cpp    | 27 ++++++
 3 files changed, 106 insertions(+), 63 deletions(-)

diff --git a/libwvdrmengine/cdm/metrics/include/value_metric.h b/libwvdrmengine/cdm/metrics/include/value_metric.h
index 5f943106..315bbe4f 100644
--- a/libwvdrmengine/cdm/metrics/include/value_metric.h
+++ b/libwvdrmengine/cdm/metrics/include/value_metric.h
@@ -15,109 +15,103 @@
 
 namespace wvcdm {
 namespace metrics {
-// Internal namespace for helper methods.
-namespace impl {
+namespace internal {
 // Helper function for setting a value in the proto.
 template <typename T>
 void SetValue(drm_metrics::ValueMetric* value_proto, const T& value);
-}  // namespace impl
+}  // namespace internal
 
 // The ValueMetric class supports storing a single, overwritable value or an
 // error code. This class can be serialized to a drm_metrics::ValueMetric proto.
 // If an error or value was not provided, this metric will serialize to nullptr.
 //
 // Example Usage:
-//   Metric<string> cdm_version().Record("a.b.c.d");
+//   ValueMetric<string> cdm_version().Record("a.b.c.d");
 //   std::unique_ptr<drm_metrics::ValueMetric> mymetric(
 //       cdm_version.ToProto());
 //
 // Example Error Usage:
 //
-//   Metric<string> cdm_version().SetError(error_code);
+//   ValueMetric<string> cdm_version().SetError(error_code);
 //   std::unique_ptr<drm_metrics::ValueMetric> mymetric(
 //       cdm_version.ToProto());
 //
 template <typename T>
 class ValueMetric {
  public:
-  // Constructs a metric with the given metric name.
-  explicit ValueMetric()
-      : error_code_(0), has_error_(false), has_value_(false) {}
+  ValueMetric() {}
 
   // Record the value of the metric.
   void Record(const T& value) {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     value_ = value;
-    has_value_ = true;
-    has_error_ = false;
+    state_ = kHasValue;
   }
 
   // Set the error code if an error was encountered.
   void SetError(int error_code) {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     error_code_ = error_code;
-    has_value_ = false;
-    has_error_ = true;
+    state_ = kHasError;
   }
 
   bool HasValue() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
-    return has_value_;
+    std::unique_lock<std::mutex> lock(mutex_);
+    return state_ == kHasValue;
   }
   const T& GetValue() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     return value_;
   }
 
   bool HasError() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
-    return has_error_;
+    std::unique_lock<std::mutex> lock(mutex_);
+    return state_ == kHasError;
   }
   int GetError() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
+    std::unique_lock<std::mutex> lock(mutex_);
     return error_code_;
   }
 
   // Clears the indicators that the metric or error was set.
   void Clear() {
-    std::unique_lock<std::mutex> lock(internal_lock_);
-    has_value_ = false;
-    has_error_ = false;
+    std::unique_lock<std::mutex> lock(mutex_);
+    state_ = kNone;
   }
 
   // Returns a new ValueMetric proto containing the metric value or the
   // error code. If neither the error or value are set, it returns nullptr.
   drm_metrics::ValueMetric* ToProto() const {
-    std::unique_lock<std::mutex> lock(internal_lock_);
-    if (has_error_) {
-      drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
-      value_proto->set_error_code(error_code_);
-      return value_proto;
-    } else if (has_value_) {
-      drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
-      impl::SetValue(value_proto, value_);
-      return value_proto;
+    std::unique_lock<std::mutex> lock(mutex_);
+    switch (state_) {
+      case kHasValue: {
+        drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
+        internal::SetValue(value_proto, value_);
+        return value_proto;
+      }
+      case kHasError: {
+        drm_metrics::ValueMetric* value_proto = new drm_metrics::ValueMetric;
+        value_proto->set_error_code(error_code_);
+        return value_proto;
+      }
+      case kNone:
+      default:
+        return nullptr;
     }
-
-    return nullptr;
   }
 
  private:
-  T value_;
-  int error_code_;
-  bool has_error_;
-  bool has_value_;
+  enum ValueState { kNone, kHasValue, kHasError };
 
-  /*
-   * This locks the internal state of the value metric to ensure safety
-   * across multiple threads preventing the caller from worrying about
-   * locking.
-   *
-   * This field must be declared mutable because the lock must be takeable even
-   * in const methods.
-   */
-  mutable std::mutex internal_lock_;
-};  // class ValueMetric
+  T value_;
+  int error_code_ = 0;
+  ValueState state_ = kNone;
+
+  // This locks the internal state of the value metric to ensure safety
+  // across multiple threads preventing the caller from worrying about
+  // locking.
+  mutable std::mutex mutex_;
+};
 }  // namespace metrics
 }  // namespace wvcdm
 #endif  // WVCDM_METRICS_VALUE_METRIC_H_
diff --git a/libwvdrmengine/cdm/metrics/src/value_metric.cpp b/libwvdrmengine/cdm/metrics/src/value_metric.cpp
index 81a2e654..a0e000c3 100644
--- a/libwvdrmengine/cdm/metrics/src/value_metric.cpp
+++ b/libwvdrmengine/cdm/metrics/src/value_metric.cpp
@@ -6,89 +6,111 @@
 // ValueMetric class.
 #include "value_metric.h"
 
+#include <limits>
+
 #include "OEMCryptoCENC.h"
 #include "metrics_collections.h"
 #include "wv_cdm_types.h"
 
 namespace wvcdm {
 namespace metrics {
-namespace impl {
+namespace internal {
+namespace {
+// Max value of protobuf's int64 type.
+constexpr uint64_t kMaxValueCmp =
+    static_cast<uint64_t>(std::numeric_limits<int64_t>::max());
+constexpr int64_t kMaxValue = std::numeric_limits<int64_t>::max();
+
+template <typename T>
+constexpr bool WillOverflow(T value) {
+  return static_cast<uint64_t>(value) > kMaxValueCmp;
+}
+
+// Prevents an overflow of very large numbers.  If the value attempted to
+// be assigned is greater than the maximum value supported by protobuf's
+// int64 type, then the value is capped at the max.
+template <typename T>
+constexpr int64_t SafeAssign(T value) {
+  return WillOverflow(value) ? kMaxValue : static_cast<int64_t>(value);
+}
+}  // namespace
+
 template <>
 void SetValue<int>(drm_metrics::ValueMetric* value_proto, const int& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<long>(drm_metrics::ValueMetric* value_proto, const long& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<long long>(drm_metrics::ValueMetric* value_proto,
                          const long long& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned int>(drm_metrics::ValueMetric* value_proto,
                             const unsigned int& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned short>(drm_metrics::ValueMetric* value_proto,
                               const unsigned short& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned long>(drm_metrics::ValueMetric* value_proto,
                              const unsigned long& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<unsigned long long>(drm_metrics::ValueMetric* value_proto,
                                   const unsigned long long& value) {
-  value_proto->set_int_value((int64_t)value);
+  value_proto->set_int_value(SafeAssign(value));
 }
 
 template <>
 void SetValue<bool>(drm_metrics::ValueMetric* value_proto, const bool& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(value ? 1 : 0);
 }
 
 template <>
 void SetValue<OEMCrypto_HDCP_Capability>(
     drm_metrics::ValueMetric* value_proto,
     const OEMCrypto_HDCP_Capability& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<OEMCrypto_ProvisioningMethod>(
     drm_metrics::ValueMetric* value_proto,
     const OEMCrypto_ProvisioningMethod& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<OEMCryptoInitializationMode>(
     drm_metrics::ValueMetric* value_proto,
     const OEMCryptoInitializationMode& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<CdmSecurityLevel>(drm_metrics::ValueMetric* value_proto,
                                 const CdmSecurityLevel& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
 void SetValue<CdmUsageSupportType>(drm_metrics::ValueMetric* value_proto,
                                    const CdmUsageSupportType& value) {
-  value_proto->set_int_value(value);
+  value_proto->set_int_value(static_cast<int64_t>(value));
 }
 
 template <>
@@ -102,6 +124,6 @@ void SetValue<std::string>(drm_metrics::ValueMetric* value_proto,
                            const std::string& value) {
   value_proto->set_string_value(value);
 }
-}  // namespace impl
+}  // namespace internal
 }  // namespace metrics
 }  // namespace wvcdm
diff --git a/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp b/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp
index 6e86f0cc..72e6fafe 100644
--- a/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp
+++ b/libwvdrmengine/cdm/metrics/test/value_metric_unittest.cpp
@@ -57,5 +57,32 @@ TEST(ValueMetricTest, SetError) {
   ASSERT_EQ(7, metric_proto->error_code());
   ASSERT_FALSE(metric_proto->has_int_value());
 }
+
+TEST(ValueMetricTest, ValueOverflow) {
+  constexpr int64_t kMaxValue = std::numeric_limits<int64_t>::max();
+  constexpr uint64_t kMaxU64 = std::numeric_limits<uint64_t>::max();
+  constexpr size_t kMaxSizeT = std::numeric_limits<size_t>::max();
+
+  ValueMetric<uint64_t> metric_u64;
+  metric_u64.Record(kMaxU64);
+  // Runtime value should not be capped.
+  ASSERT_EQ(kMaxU64, metric_u64.GetValue());
+
+  std::unique_ptr<drm_metrics::ValueMetric> metric_proto(metric_u64.ToProto());
+  // Proto value should be capped.
+  ASSERT_EQ(kMaxValue, metric_proto->int_value());
+  ASSERT_FALSE(metric_proto->has_error_code());
+
+  ValueMetric<uint64_t> metric_z;
+  metric_z.Record(kMaxSizeT);
+  ASSERT_EQ(kMaxSizeT, metric_z.GetValue());
+  metric_proto.reset(metric_z.ToProto());
+  if (sizeof(int64_t) <= sizeof(size_t) &&
+      static_cast<size_t>(kMaxValue) <= kMaxSizeT) {
+    ASSERT_EQ(kMaxValue, metric_proto->int_value());
+  } else {
+    ASSERT_GT(kMaxValue, metric_proto->int_value());
+  }
+}
 }  // namespace metrics
 }  // namespace wvcdm