moqt: avoid int64 overflow converting peer millisecond varints to QuicTimeDelta

quiche/quic/moqt/moqt_key_value_pair.cc

@@ -5,6 +5,7 @@
 #include "quiche/quic/moqt/moqt_key_value_pair.h"
 
 #include <cstdint>
+#include <limits>
 #include <optional>
 #include <string>
 #include <variant>
@@ -19,6 +20,20 @@
 
 namespace moqt {
 
+namespace {
+
+// Converts a peer-supplied millisecond count to a QuicTimeDelta. Values that
+// don't fit in int64 microseconds collapse to an infinite delta.
+quic::QuicTimeDelta MillisecondsToTimeDelta(uint64_t ms) {
+  if (ms > static_cast<uint64_t>(std::numeric_limits<int64_t>::max())) {
+    return quic::QuicTimeDelta::Infinite();
+  }
+  return quic::QuicTimeDelta::TryFromMilliseconds(static_cast<int64_t>(ms))
+      .value_or(quic::QuicTimeDelta::Infinite());
+}
+
+}  // namespace
+
 void KeyValuePairList::insert(uint64_t key,
                               std::variant<uint64_t, absl::string_view> value) {
   QUICHE_DCHECK(
@@ -158,20 +173,21 @@ quic::QuicTimeDelta TrackProperties::object_delivery_timeout() const {
   std::optional<uint64_t> value =
       GetValueIfExactlyOne(PropertyType::kObjectDeliveryTimeout);
   return (value.has_value() && *value > 0)
-             ? quic::QuicTimeDelta::FromMilliseconds(*value)
+             ? MillisecondsToTimeDelta(*value)
              : kDefaultDeliveryTimeout;
 }
 quic::QuicTimeDelta TrackProperties::max_cache_duration() const {
   std::optional<uint64_t> value =
       GetValueIfExactlyOne(PropertyType::kMaxCacheDuration);
-  return value.has_value() ? quic::QuicTimeDelta::FromMilliseconds(*value)
-                           : kDefaultMaxCacheDuration;
+  return value.has_value()
+             ? MillisecondsToTimeDelta(*value)
+             : kDefaultMaxCacheDuration;
 }
 quic::QuicTimeDelta TrackProperties::subgroup_delivery_timeout() const {
   std::optional<uint64_t> value =
       GetValueIfExactlyOne(PropertyType::kSubgroupDeliveryTimeout);
   return (value.has_value() && *value > 0)
-             ? quic::QuicTimeDelta::FromMilliseconds(*value)
+             ? MillisecondsToTimeDelta(*value)
              : kDefaultDeliveryTimeout;
 }
 absl::string_view TrackProperties::immutable_properties() const {

quiche/quic/moqt/moqt_key_value_pair_test.cc

@@ -210,6 +210,32 @@ TEST_F(TrackPropertiesTest, ExplicitDefaults) {
   EXPECT_TRUE(properties.immutable_properties().empty());
 }
 
+TEST_F(TrackPropertiesTest, OversizedTimeoutsDoNotOverflow) {
+  // Timeout properties carry a peer-supplied millisecond value that is turned
+  // into microseconds by a multiply by 1000. Values whose conversion would
+  // overflow int64 must collapse to an infinite delta rather than trigger
+  // signed overflow. Values above INT64_MAX must not wrap to a negative
+  // delta. Validate() places no upper bound on these keys.
+  for (uint64_t overflowing_ms : {uint64_t{1} << 62, UINT64_MAX}) {
+    TrackProperties properties;
+    properties.insert(
+        static_cast<uint64_t>(PropertyType::kObjectDeliveryTimeout),
+        overflowing_ms);
+    properties.insert(static_cast<uint64_t>(PropertyType::kMaxCacheDuration),
+                      overflowing_ms);
+    properties.insert(
+        static_cast<uint64_t>(PropertyType::kSubgroupDeliveryTimeout),
+        overflowing_ms);
+    EXPECT_TRUE(properties.Validate());
+    EXPECT_EQ(properties.object_delivery_timeout(),
+              quic::QuicTimeDelta::Infinite());
+    EXPECT_EQ(properties.max_cache_duration(),
+              quic::QuicTimeDelta::Infinite());
+    EXPECT_EQ(properties.subgroup_delivery_timeout(),
+              quic::QuicTimeDelta::Infinite());
+  }
+}
+
 TEST_F(TrackPropertiesTest, Validate) {
   TrackProperties properties;
   // Unknown property.

quiche/quic/moqt/moqt_parser.cc

@@ -756,7 +756,12 @@ absl::StatusOr<MoqtRequestError> MoqtControlMessageParser::ProcessRequestError(
       (raw_interval == 0)
           ? std::nullopt
           : std::make_optional(
-                quic::QuicTimeDelta::FromMilliseconds(raw_interval - 1));
+                (raw_interval - 1 >
+                 static_cast<uint64_t>(std::numeric_limits<int64_t>::max()))
+                    ? quic::QuicTimeDelta::Infinite()
+                    : quic::QuicTimeDelta::TryFromMilliseconds(
+                          static_cast<int64_t>(raw_interval - 1))
+                          .value_or(quic::QuicTimeDelta::Infinite()));
   if (request_error.error_code == RequestErrorCode::kRedirect) {
     request_error.redirect.emplace();
     if (!reader.ReadStringMoqVarInt(request_error.redirect->connect_uri)) {

quiche/quic/moqt/moqt_parser_test.cc

@@ -2403,4 +2403,45 @@ TEST_F(MoqtMessageSpecificTest, RequestErrorRedirect) {
                                HasSubstr("Control message has excess data")));
 }
 
+TEST_F(MoqtMessageSpecificTest, RequestErrorOversizedRetryInterval) {
+  // retry_interval is a peer-supplied millisecond varint that is converted to
+  // microseconds (a multiply by 1000). A value whose conversion overflows
+  // int64 must collapse to an infinite delta rather than trigger signed
+  // overflow.
+  char request_error[] = {
+      0x05, 0x00, 0x0b,                          // type = REQUEST_ERROR, size
+      0x11,                                      // error_code (non-redirect)
+      0xff, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00,  // retry_interval = 2^60 as a
+      0x00, 0x00,                                //   9-byte MoqVarInt
+      0x00,                                      // reason_phrase = ""
+  };
+  absl::StatusOr<std::vector<AnyMoqtControlMessage>> parsed = ParseAllMessages(
+      absl::string_view(request_error, sizeof(request_error)),
+      kDefaultMoqtVersion, kWebTrans, quic::Perspective::IS_CLIENT);
+  ASSERT_TRUE(parsed.ok()) << parsed.status();
+  ASSERT_EQ(parsed->size(), 1u);
+  const MoqtRequestError& error = std::get<MoqtRequestError>((*parsed)[0]);
+  ASSERT_TRUE(error.retry_interval.has_value());
+  EXPECT_EQ(*error.retry_interval, quic::QuicTimeDelta::Infinite());
+}
+
+TEST_F(MoqtMessageSpecificTest, RequestErrorMaxRetryIntervalDoesNotWrap) {
+  // A retry_interval above INT64_MAX must not wrap to a negative delta.
+  char request_error[] = {
+      0x05, 0x00, 0x0b,                          // type = REQUEST_ERROR, size
+      0x11,                                      // error_code (non-redirect)
+      0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,  // retry_interval = UINT64_MAX
+      0xff, 0xff,                                //   as a 9-byte MoqVarInt
+      0x00,                                      // reason_phrase = ""
+  };
+  absl::StatusOr<std::vector<AnyMoqtControlMessage>> parsed = ParseAllMessages(
+      absl::string_view(request_error, sizeof(request_error)),
+      kDefaultMoqtVersion, kWebTrans, quic::Perspective::IS_CLIENT);
+  ASSERT_TRUE(parsed.ok()) << parsed.status();
+  ASSERT_EQ(parsed->size(), 1u);
+  const MoqtRequestError& error = std::get<MoqtRequestError>((*parsed)[0]);
+  ASSERT_TRUE(error.retry_interval.has_value());
+  EXPECT_EQ(*error.retry_interval, quic::QuicTimeDelta::Infinite());
+}
+
 }  // namespace moqt::test