From 52b9e1ecfbc83e92cee06594d18febc0d1f944d7 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Mon, 13 Sep 2021 12:59:38 +0200 Subject: [PATCH] Ensure RtpVideoLayersAllocationExtension::Parse validate sanity of the output This is tested by a simple unit test and a new fuzzer that verify that all that can be parsed also can be written. Bug: webrtc:12000 Change-Id: I461aedf97d3dec6e8916e72110fa097c3b31c27f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231642 Reviewed-by: Sam Zackrisson Reviewed-by: Danil Chapovalov Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#34986} --- .../rtp_video_layers_allocation_extension.cc | 7 ++- ...eo_layers_allocation_extension_unittest.cc | 27 +++++++++ test/fuzzers/BUILD.gn | 11 ++++ .../video_layers_allocation-corpus/vla-0 | 1 + .../rtp_video_layers_allocation_fuzzer.cc | 57 +++++++++++++++++++ 5 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 test/fuzzers/corpora/video_layers_allocation-corpus/vla-0 create mode 100644 test/fuzzers/rtp_video_layers_allocation_fuzzer.cc diff --git a/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension.cc b/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension.cc index 234ac31b8b..1463cf40d6 100644 --- a/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension.cc +++ b/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension.cc @@ -289,7 +289,7 @@ bool RtpVideoLayersAllocationExtension::Parse( if (data.size() == 1 && *read_at == 0) { allocation->rtp_stream_index = 0; allocation->resolution_and_frame_rate_is_valid = true; - return true; + return AllocationIsValid(*allocation); } // Header byte. @@ -366,7 +366,7 @@ bool RtpVideoLayersAllocationExtension::Parse( if (read_at == end) { allocation->resolution_and_frame_rate_is_valid = false; - return true; + return AllocationIsValid(*allocation); } if (read_at + 5 * allocation->active_spatial_layers.size() != end) { @@ -383,7 +383,8 @@ bool RtpVideoLayersAllocationExtension::Parse( layer.frame_rate_fps = *read_at; ++read_at; } - return true; + + return AllocationIsValid(*allocation); } size_t RtpVideoLayersAllocationExtension::ValueSize( diff --git a/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension_unittest.cc b/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension_unittest.cc index 17b4c4cfa6..db077409ee 100644 --- a/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_video_layers_allocation_extension_unittest.cc @@ -256,5 +256,32 @@ TEST(RtpVideoLayersAllocationExtension, DiscardsOverLargeDataRate) { EXPECT_FALSE(RtpVideoLayersAllocationExtension::Parse(buffer, &allocation)); } +TEST(RtpVideoLayersAllocationExtension, DiscardsInvalidHeight) { + VideoLayersAllocation written_allocation; + written_allocation.rtp_stream_index = 0; + written_allocation.resolution_and_frame_rate_is_valid = true; + written_allocation.active_spatial_layers = { + { + /*rtp_stream_index*/ 0, + /*spatial_id*/ 0, + /*target_bitrate_per_temporal_layer*/ + {DataRate::KilobitsPerSec(25), DataRate::KilobitsPerSec(50)}, + /*width*/ 320, + /*height*/ 240, + /*frame_rate_fps*/ 8, + }, + }; + rtc::Buffer buffer( + RtpVideoLayersAllocationExtension::ValueSize(written_allocation)); + ASSERT_TRUE( + RtpVideoLayersAllocationExtension::Write(buffer, written_allocation)); + + // Modify the height to be invalid. + buffer[buffer.size() - 3] = 0xff; + buffer[buffer.size() - 2] = 0xff; + VideoLayersAllocation allocation; + EXPECT_FALSE(RtpVideoLayersAllocationExtension::Parse(buffer, &allocation)); +} + } // namespace } // namespace webrtc diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index 019b845727..b024ce2dc6 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -559,6 +559,17 @@ webrtc_fuzzer_test("rtp_dependency_descriptor_fuzzer") { ] } +webrtc_fuzzer_test("rtp_video_layers_allocation_fuzzer") { + sources = [ "rtp_video_layers_allocation_fuzzer.cc" ] + seed_corpus = "corpora/video_layers_allocation-corpus" + deps = [ + "../../api:array_view", + "../../api/video:video_layers_allocation", + "../../modules/rtp_rtcp:rtp_rtcp_format", + "../../rtc_base:checks", + ] +} + webrtc_fuzzer_test("rtp_frame_reference_finder_fuzzer") { sources = [ "rtp_frame_reference_finder_fuzzer.cc" ] deps = [ diff --git a/test/fuzzers/corpora/video_layers_allocation-corpus/vla-0 b/test/fuzzers/corpora/video_layers_allocation-corpus/vla-0 new file mode 100644 index 0000000000..1b6fdf78f6 --- /dev/null +++ b/test/fuzzers/corpora/video_layers_allocation-corpus/vla-0 @@ -0,0 +1 @@ +@2?ÿÿ \ No newline at end of file diff --git a/test/fuzzers/rtp_video_layers_allocation_fuzzer.cc b/test/fuzzers/rtp_video_layers_allocation_fuzzer.cc new file mode 100644 index 0000000000..8e203bc1c4 --- /dev/null +++ b/test/fuzzers/rtp_video_layers_allocation_fuzzer.cc @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2021 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include +#include + +#include "api/array_view.h" +#include "api/video/video_layers_allocation.h" +#include "modules/rtp_rtcp/source/rtp_video_layers_allocation_extension.h" +#include "rtc_base/checks.h" + +namespace webrtc { + +void FuzzOneInput(const uint8_t* data, size_t size) { + auto raw = rtc::MakeArrayView(data, size); + + VideoLayersAllocation allocation1; + if (!RtpVideoLayersAllocationExtension::Parse(raw, &allocation1)) { + // Ignore invalid buffer and move on. + return; + } + + // Write parsed allocation back into raw buffer. + size_t value_size = RtpVideoLayersAllocationExtension::ValueSize(allocation1); + // Check `writer` use minimal number of bytes to pack the extension by + // checking it doesn't use more than reader consumed. + RTC_CHECK_LE(value_size, raw.size()); + uint8_t some_memory[256]; + // An extension may not be larger than 255 bytes since the extension lenght + // field is only one byte. + RTC_CHECK_LT(value_size, 256); + rtc::ArrayView write_buffer(some_memory, value_size); + RTC_CHECK( + RtpVideoLayersAllocationExtension::Write(write_buffer, allocation1)); + + // Parse what Write assembled. + // Unlike random input that should always succeed. + VideoLayersAllocation allocation2; + RTC_CHECK( + RtpVideoLayersAllocationExtension::Parse(write_buffer, &allocation2)); + + RTC_CHECK_EQ(allocation1.rtp_stream_index, allocation2.rtp_stream_index); + RTC_CHECK_EQ(allocation1.resolution_and_frame_rate_is_valid, + allocation2.resolution_and_frame_rate_is_valid); + RTC_CHECK_EQ(allocation1.active_spatial_layers.size(), + allocation2.active_spatial_layers.size()); + RTC_CHECK(allocation1 == allocation2); +} + +} // namespace webrtc