Check that the audio_frame_ is not resized to a negative number

If the payload_size is less than the number of bytes used by the substream ID, then we should fail. PiperOrigin-RevId: 714120666
AOMediaCodec · Jan 13, 2025 · b05301b · b05301b
1 parent 9a9b0a4
commit b05301b
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 9 deletions.
diff --git a/iamf/obu/BUILD b/iamf/obu/BUILD
@@ -58,6 +58,7 @@ cc_library(
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings",
         "@com_google_absl//absl/types:span",
     ],
 )

diff --git a/iamf/obu/audio_frame.cc b/iamf/obu/audio_frame.cc
@@ -16,6 +16,7 @@
 
 #include "absl/log/log.h"
 #include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
 #include "absl/types/span.h"
 #include "iamf/common/macros.h"
 #include "iamf/common/read_bit_buffer.h"
@@ -76,7 +77,11 @@ absl::Status AudioFrameObu::ReadAndValidatePayloadDerived(int64_t payload_size,
   } else {
     audio_substream_id_ = header_.obu_type - kObuIaAudioFrameId0;
   }
-
+  if (payload_size < 0 || payload_size < encoded_uleb128_size) {
+    return absl::InvalidArgumentError(absl::StrCat(
+        "Less than zero bytes remaining in payload. payload_size=",
+        payload_size, " encoded_uleb128_size=", encoded_uleb128_size));
+  }
   audio_frame_.resize(payload_size - encoded_uleb128_size);
   return rb.ReadUint8Span(absl::MakeSpan(audio_frame_));
 }

diff --git a/iamf/obu/tests/audio_frame_test.cc b/iamf/obu/tests/audio_frame_test.cc
@@ -293,13 +293,13 @@ TEST_F(AudioFrameObuTest, ValidateAndWriteObuFailsWithIllegalRedundantCopy) {
 
 // --- Begin CreateFromBuffer tests ---
 TEST(CreateFromBuffer, ValidAudioFrameWithExplicitId) {
-  std::vector<uint8_t> source = {// `explicit_audio_substream_id`
+  std::vector<uint8_t> source = {// `explicit_audio_substream_id`, arbitrary.
                                  18,
-                                 // `audio_frame`.
+                                 // `audio_frame`, arbitrary values.
                                  8, 6, 24, 55, 11};
   auto buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
       1024, absl::MakeConstSpan(source));
-  ObuHeader header = {.obu_type = kObuIaAudioFrame};
+  ObuHeader header = {.obu_type = kObuIaAudioFrame};  // Requires explicit ID.
   const int64_t obu_payload_size = 6;
   auto obu = AudioFrameObu::CreateFromBuffer(header, obu_payload_size, *buffer);
   EXPECT_THAT(obu, IsOk());
@@ -308,11 +308,11 @@ TEST(CreateFromBuffer, ValidAudioFrameWithExplicitId) {
 }
 
 TEST(CreateFromBuffer, ValidAudioFrameWithImplicitId) {
-  std::vector<uint8_t> source = {// `audio_frame`.
+  std::vector<uint8_t> source = {// `audio_frame`, arbitrary values.
                                  8, 6, 24, 55, 11};
   auto buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
       1024, absl::MakeConstSpan(source));
-  ObuHeader header = {.obu_type = kObuIaAudioFrameId0};
+  ObuHeader header = {.obu_type = kObuIaAudioFrameId0};  // ID from OBU type.
   int64_t obu_payload_size = 5;
   auto obu = AudioFrameObu::CreateFromBuffer(header, obu_payload_size, *buffer);
   EXPECT_THAT(obu, IsOk());
@@ -325,17 +325,55 @@ TEST(CreateFromBuffer, ValidAudioFrameWithImplicitId) {
 }
 
 TEST(CreateFromBuffer, FailsWithPayloadSizeTooLarge) {
-  std::vector<uint8_t> source = {// `explicit_audio_substream_id`
+  std::vector<uint8_t> source = {// `explicit_audio_substream_id`, arbitrary.
                                  18,
-                                 // `audio_frame`.
+                                 // `audio_frame`, arbitrary values.
                                  8, 6, 24, 55, 11};
   auto buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
       1024, absl::MakeConstSpan(source));
-  ObuHeader header = {.obu_type = kObuIaAudioFrame};
+  ObuHeader header = {.obu_type = kObuIaAudioFrame};  // Requires explicit ID.
   int64_t obu_payload_size = 7;
   auto obu = AudioFrameObu::CreateFromBuffer(header, obu_payload_size, *buffer);
   EXPECT_FALSE(obu.ok());
 }
 
+TEST(CreateFromBuffer, FailsWithPayloadSizeZero) {
+  // With one byte used by the substream ID, a payload size of 0 is not valid.
+  std::vector<uint8_t> source = {// `explicit_audio_substream_id`, arbitrary.
+                                 18,
+                                 // `audio_frame`, arbitrary values.
+                                 8, 6, 24, 55, 11};
+  auto buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
+      1024, absl::MakeConstSpan(source));
+  ObuHeader header = {.obu_type = kObuIaAudioFrame};  // Requires explicit ID.
+  int64_t obu_payload_size = 0;
+  auto obu = AudioFrameObu::CreateFromBuffer(header, obu_payload_size, *buffer);
+  EXPECT_FALSE(obu.ok());
+}
+
+TEST(CreateFromBuffer, FailsWithPayloadSizeLessThanSizeUsedById) {
+  // With three bytes used by the ID, a payload size of 2 is not valid.
+  std::vector<uint8_t> source = {
+      0x80, 0x80, 0x1,           // `explicit_audio_substream_id`, 3 byte ULEB.
+      8,    6,    24,  55, 11};  // `audio_frame`, arbitrary values.
+  auto buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
+      1024, absl::MakeConstSpan(source));
+  ObuHeader header = {.obu_type = kObuIaAudioFrame};  // Requires explicit ID.
+  int64_t obu_payload_size = 2;  // Less than the 3 used for the ID.
+  auto obu = AudioFrameObu::CreateFromBuffer(header, obu_payload_size, *buffer);
+  EXPECT_FALSE(obu.ok());
+}
+
+TEST(CreateFromBuffer, FailsWithNegativePayloadSize) {
+  std::vector<uint8_t> source = {// `audio_frame`, arbitrary values.
+                                 8, 6, 24, 55, 11};
+  auto buffer = MemoryBasedReadBitBuffer::CreateFromSpan(
+      1024, absl::MakeConstSpan(source));
+  ObuHeader header = {.obu_type = kObuIaAudioFrameId0};  // ID from OBU type.
+  int64_t obu_payload_size = -1;
+  auto obu = AudioFrameObu::CreateFromBuffer(header, obu_payload_size, *buffer);
+  EXPECT_FALSE(obu.ok());
+}
+
 }  // namespace
 }  // namespace iamf_tools