From 8767d20f38110ca11023f4076585f91b25808524 Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Tue, 24 Apr 2018 14:28:38 -0700 Subject: [PATCH] Fix potential slice header size off by one byte in H265 In H265Parser::ParseSliceHeader, the parser does not handle byte_alignment() from the spec. byte_alignment() reportedly contains at least one bit, which is not handled right now. See Section 7.3.2.12 Rec. ITU-T H.265 v3 (04/2015). Also added a few size sanity checks in H265Parser to make sure the code does not crash if an invalid input is provided. Fixes #383. Change-Id: I33b31396058fc5ba67a0fc119be5fe56ec9443b0 --- .../bear-640x360-hevc-video.mp4 | Bin 92801 -> 92801 bytes packager/media/codecs/h265_parser.cc | 12 ++++++++++++ packager/media/codecs/h265_parser.h | 2 ++ packager/media/codecs/h265_parser_unittest.cc | 4 ++-- .../media/codecs/h26x_bit_reader_unittest.cc | 2 ++ 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packager/app/test/testdata/hevc-with-encryption/bear-640x360-hevc-video.mp4 b/packager/app/test/testdata/hevc-with-encryption/bear-640x360-hevc-video.mp4 index b98897e81a24c551b50799e7fc144a2d52d08b0a..e838224228737148e9dbebf9bfd0b2447cdf456e 100644 GIT binary patch delta 417 zcmV;S0bc%r)dhjo1%R{xSOEnh0004Sw_O1N?Js|TIz(@~kS1vmQ;U26R6l{a1K(dWCW+M`?2nU4ueYA?iS?t*~!a~1-7JBFj0}|ACiVim(;aRK)A3*kgWSy{rBi#?Ev|6Fx zK?(3OEQ6mnhJ3apB@!1bv_)h;29-|C|6@rNXy%=aJ@jqXUQsSOhZ&r}$#4RCBd_jv z;(E<)M?H{RIg*k|4vo~`??h0<2LeVz%r}3RzqVEw?3jd$3z_b`7N{b~9vpGb*b}0l zn#t9NLm;hiLr1V$dQ%eP`v+!HedwNJ;#Z(`)|Wa+fV^Xvqb$QRE=v zx}Ek0Fcpx7THkOF=~vTdbaG_zQ+1Z>T}oIOgS2DcJxieUpj;-~UguBUI36B8N_qpk Lmk77YG6A==~qX^UVvDGo(7UF||^U<5P*c@SkAJ zyYTN=%eGoL*+L(ORSw*NL9Wp&-uE|!6SuJlAd*v>9vfz-W0+(_B!jAAUN14L2x+!U zB3OToPs(PZM|+BfB2lUD^oeFi5J(wV8TeYz60FZrYxLo5ERl_*zz#&Tjp=~bq@adk zB|f_k;#ocwd*l7rXKUU6#S>fMwx^iL(7|^0Zy*PMFzifmlOwSUYWJ_86xpgAw10XTX2Dd`1Nr#yGzO(NeuN8k=Qh^o-HPoTU9( z629~;h%YUC@{Rt<@q}cAL~y*KCHGjEjUuDUu`xWYtAU{&4bCiasWbn`3i=D1TFJsz z2IP<;o95syIKj~!^pDmK1@vw$(Wv-H0uJ2Qk5;hUF~fd?*gXa~1U}l~{|LhJC^`dz LVOY1yG6AReadBits(ceil(log2(sps->num_short_term_ref_pic_sets)), &slice_header->short_term_ref_pic_set_idx)); + TRUE_OR_RETURN(slice_header->short_term_ref_pic_set_idx < + sps->num_short_term_ref_pic_sets); } if (sps->long_term_ref_pic_present_flag) { @@ -390,6 +392,8 @@ H265Parser::Result H265Parser::ParseSliceHeader(const Nalu& nalu, TRUE_OR_RETURN(br->SkipBits(extension_length * 8)); } + OK_OR_RETURN(ByteAlignment(br)); + slice_header->header_bit_size = nalu.payload_size() * 8 - br->NumBitsLeft(); return kOk; } @@ -832,7 +836,9 @@ H265Parser::Result H265Parser::ParseReferencePictureSet( } } else { TRUE_OR_RETURN(br->ReadUE(&out_ref_pic_set->num_negative_pics)); + TRUE_OR_RETURN(out_ref_pic_set->num_negative_pics <= kMaxRefPicSetCount); TRUE_OR_RETURN(br->ReadUE(&out_ref_pic_set->num_positive_pics)); + TRUE_OR_RETURN(out_ref_pic_set->num_positive_pics <= kMaxRefPicSetCount); int prev_poc = 0; for (int i = 0; i < out_ref_pic_set->num_negative_pics; i++) { @@ -1107,5 +1113,11 @@ H265Parser::Result H265Parser::SkipSubLayerHrdParameters( return kOk; } +H265Parser::Result H265Parser::ByteAlignment(H26xBitReader* br) { + TRUE_OR_RETURN(br->SkipBits(1)); + TRUE_OR_RETURN(br->SkipBits(br->NumBitsLeft() % 8)); + return kOk; +} + } // namespace media } // namespace shaka diff --git a/packager/media/codecs/h265_parser.h b/packager/media/codecs/h265_parser.h index 5b0253a33f..6eae120d1a 100644 --- a/packager/media/codecs/h265_parser.h +++ b/packager/media/codecs/h265_parser.h @@ -351,6 +351,8 @@ class H265Parser { bool sub_pic_hdr_params_present_flag, H26xBitReader* br); + Result ByteAlignment(H26xBitReader* br); + typedef std::map> SpsById; typedef std::map> PpsById; diff --git a/packager/media/codecs/h265_parser_unittest.cc b/packager/media/codecs/h265_parser_unittest.cc index 55ec82f9b8..063f2f5a8b 100644 --- a/packager/media/codecs/h265_parser_unittest.cc +++ b/packager/media/codecs/h265_parser_unittest.cc @@ -55,7 +55,7 @@ TEST(H265ParserTest, ParseSliceHeader) { EXPECT_EQ(8, header.slice_qp_delta); EXPECT_FALSE(header.cu_chroma_qp_offset_enabled_flag); EXPECT_EQ(5, header.num_entry_point_offsets); - EXPECT_EQ(85u, header.header_bit_size); + EXPECT_EQ(88u, header.header_bit_size); } TEST(H265ParserTest, ParseSliceHeader_NonIDR) { @@ -81,7 +81,7 @@ TEST(H265ParserTest, ParseSliceHeader_NonIDR) { EXPECT_FALSE(header.dependent_slice_segment_flag); EXPECT_EQ(1, header.slice_type); EXPECT_EQ(5, header.num_entry_point_offsets); - EXPECT_EQ(124u, header.header_bit_size); + EXPECT_EQ(128u, header.header_bit_size); } TEST(H265ParserTest, ParseSps) { diff --git a/packager/media/codecs/h26x_bit_reader_unittest.cc b/packager/media/codecs/h26x_bit_reader_unittest.cc index f15b866e3f..0dff514765 100644 --- a/packager/media/codecs/h26x_bit_reader_unittest.cc +++ b/packager/media/codecs/h26x_bit_reader_unittest.cc @@ -95,6 +95,8 @@ TEST(H26xBitReaderTest, SkipBits) { EXPECT_EQ(0x15, dummy); EXPECT_EQ(4, reader.NumBitsLeft()); EXPECT_FALSE(reader.SkipBits(5)); + EXPECT_TRUE(reader.SkipBits(0)); + EXPECT_EQ(4, reader.NumBitsLeft()); } TEST(H26xBitReaderTest, StopBitOccupyFullByte) {