Buffer over-read causes segmentation fault in pic_parameter_set::dump

litios commented 1 year ago

Summary

There is a segmentation fault caused by a buffer over-read on pic_parameter_set::dump due to an incorrect value of num_tile_columns or num_tile_rows.

Tested with:

Commit: 6fc550faeca2bbb1c82de5f29d970f6861146862 (master)
PoC: poc
cmd: ./dec265 -d poc

Crash output:

INFO: ----------------- SPS -----------------
INFO: video_parameter_set_id  : 0
INFO: sps_max_sub_layers      : 1
INFO: sps_temporal_id_nesting_flag : 1
INFO:   general_profile_space     : 0
INFO:   general_tier_flag         : 0
INFO:   general_profile_idc       : Main
INFO:   general_profile_compatibility_flags: 0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
INFO:     general_progressive_source_flag : 0
INFO:     general_interlaced_source_flag : 0
INFO:     general_non_packed_constraint_flag : 0
INFO:     general_frame_only_constraint_flag : 0
INFO:   general_level_idc         : 60 (2.00)
INFO: seq_parameter_set_id    : 0
INFO: chroma_format_idc       : 1 (4:2:0)
INFO: pic_width_in_luma_samples  : 416
INFO: pic_height_in_luma_samples : 240
INFO: conformance_window_flag    : 1
INFO: conf_win_left_offset  : 0
INFO: conf_win_right_offset : 0
INFO: conf_win_top_offset   : 0
INFO: conf_win_bottom_offset: 0
INFO: bit_depth_luma   : 10
INFO: bit_depth_chroma : 9
INFO: log2_max_pic_order_cnt_lsb : 4
INFO: sps_sub_layer_ordering_info_present_flag : 1
INFO: Layer 0
INFO:   sps_max_dec_pic_buffering      : 5
INFO:   sps_max_num_reorder_pics       : 3
INFO:   sps_max_latency_increase_plus1 : 0
INFO: log2_min_luma_coding_block_size : 3
INFO: log2_diff_max_min_luma_coding_block_size : 1
INFO: log2_min_transform_block_size   : 2
INFO: log2_diff_max_min_transform_block_size : 2
INFO: max_transform_hierarchy_depth_inter : 0
INFO: max_transform_hierarchy_depth_intra : 0
INFO: scaling_list_enable_flag : 0
INFO: amp_enabled_flag                    : 1
INFO: sample_adaptive_offset_enabled_flag : 1
INFO: pcm_enabled_flag                    : 0
INFO: num_short_term_ref_pic_sets : 11
INFO: ref_pic_set[  0 ]: X...X.X.X.......|................
INFO: ref_pic_set[  1 ]: ..........X.X...|...X............
INFO: ref_pic_set[  2 ]: ............X.X.|.X...X..........
INFO: ref_pic_set[  3 ]: ...............X|X.X...X.........
INFO: ref_pic_set[  4 ]: .............X.X|X...X...........
INFO: ref_pic_set[  5 ]: ..........X.X.X.|.X..............
INFO: ref_pic_set[  6 ]: ...........X...X|X.X.............
INFO: ref_pic_set[  7 ]: .............X..|X.X.X...........
INFO: ref_pic_set[  8 ]: ........X.......|................
INFO: ref_pic_set[  9 ]: ............X...|...X............
INFO: ref_pic_set[ 10 ]: ..............X.|.X...X..........
INFO: long_term_ref_pics_present_flag : 0
INFO: sps_temporal_mvp_enabled_flag      : 1
INFO: strong_intra_smoothing_enable_flag : 1
INFO: vui_parameters_present_flag        : 1
INFO: sps_extension_present_flag    : 0
INFO: sps_range_extension_flag      : 0
INFO: sps_multilayer_extension_flag : 0
INFO: sps_extension_6bits           : 0
INFO: CtbSizeY     : 16
INFO: MinCbSizeY   : 8
INFO: MaxCbSizeY   : 16
INFO: MinTBSizeY   : 4
INFO: MaxTBSizeY   : 16
INFO: PicWidthInCtbsY         : 26
INFO: PicHeightInCtbsY        : 15
INFO: SubWidthC               : 2
INFO: SubHeightC              : 2
INFO: ----------------- VUI -----------------
INFO: sample aspect ratio        : 0:0
INFO: overscan_info_present_flag : 0
INFO: overscan_appropriate_flag  : 0
INFO: video_signal_type_present_flag: 0
INFO: chroma_loc_info_present_flag: 0
INFO: neutral_chroma_indication_flag: 0
INFO: field_seq_flag                : 0
INFO: frame_field_info_present_flag : 0
INFO: default_display_window_flag   : 1
INFO:   def_disp_win_left_offset    : 0
INFO:   def_disp_win_right_offset   : 0
INFO:   def_disp_win_top_offset     : 0
INFO:   def_disp_win_bottom_offset  : 0
INFO: vui_timing_info_present_flag  : 0
INFO: vui_poc_proportional_to_timing_flag : 0
INFO: vui_num_ticks_poc_diff_one          : 1
INFO: vui_hrd_parameters_present_flag : 0
INFO: bitstream_restriction_flag         : 0
INFO: ----------------- PPS -----------------
INFO: pic_parameter_set_id       : 0
INFO: seq_parameter_set_id       : 0
INFO: dependent_slice_segments_enabled_flag : 1
INFO: sign_data_hiding_flag      : 1
INFO: cabac_init_present_flag    : 0
INFO: num_ref_idx_l0_default_active : -84
INFO: num_ref_idx_l1_default_active : 12
INFO: pic_init_qp                : 25
INFO: constrained_intra_pred_flag: 0
INFO: transform_skip_enabled_flag: 1
INFO: cu_qp_delta_enabled_flag   : 0
INFO: pic_cb_qp_offset             : 1
INFO: pic_cr_qp_offset             : 0
INFO: pps_slice_chroma_qp_offsets_present_flag : 0
INFO: weighted_pred_flag           : 0
INFO: weighted_bipred_flag         : 1
INFO: output_flag_present_flag     : 0
INFO: transquant_bypass_enable_flag: 0
INFO: tiles_enabled_flag           : 1
INFO: entropy_coding_sync_enabled_flag: 0
INFO: num_tile_columns    : 18815
INFO: num_tile_rows       : 1
INFO: uniform_spacing_flag: 1
INFO: tile column boundaries: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3985 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 [1]    4017733 segmentation fault  ./dec265 -d poc

Analysis

While executing decoder_context::read_pps_NAL, parameters are read in

bool success = new_pps->read(&reader,this);

Inside the function, there is a check when setting num_tile_columns in case it goes over DE265_MAX_TILE_COLUMNS, which is 10.

  if (tiles_enabled_flag) {
    num_tile_columns = get_uvlc(br);
    if (num_tile_columns == UVLC_ERROR ||
    num_tile_columns+1 > DE265_MAX_TILE_COLUMNS) {
      ctx->add_warning(DE265_WARNING_PPS_HEADER_INVALID, false);
      return false;
    }

After exiting due to reading more than DE265_MAX_TILE_COLUMNS, the headers are dumped by calling dump:

  bool success = new_pps->read(&reader,this);

  if (param_pps_headers_fd>=0) {
    new_pps->dump(param_pps_headers_fd);
  }

  if (success) {
    pps[ (int)new_pps->pic_parameter_set_id ] = new_pps;
  }

In dump, the following code is executed to dump the tile column boundaries:

    LOG0("tile column boundaries: ");
    for (int i=0;i<=num_tile_columns;i++) {
      LOG1("*%d ",colBd[i]);
    }
    LOG0("*\n");

As previously shown, num_tile_columns while be set to a higher number than DE265_MAX_TILE_COLUMNS. colBd is defined as:

int colBd [ DE265_MAX_TILE_COLUMNS+1 ];

Therefore, that loop will go over colBd and will print all the data pointed by the values found after the limits of colBd in memory until the end of the loop or the next memory address is invalid.

Impact

Information leak from memory
Crash

If using a carefully crafted exploit, the impact could be an information leak without a crash.

Patch

In order to prevent this, the success value should be checked before printing the information:

diff --git a/libde265/decctx.cc b/libde265/decctx.cc
index 223a6aaf..1b79adec 100644
--- a/libde265/decctx.cc
+++ b/libde265/decctx.cc
@@ -583,11 +583,10 @@ de265_error decoder_context::read_pps_NAL(bitreader& reader)

   bool success = new_pps->read(&reader,this);

-  if (param_pps_headers_fd>=0) {
-    new_pps->dump(param_pps_headers_fd);
-  }
-
   if (success) {
+    if (param_pps_headers_fd>=0) {
+      new_pps->dump(param_pps_headers_fd);
+    }
     pps[ (int)new_pps->pic_parameter_set_id ] = new_pps;
   }

Another possibility could be to perform length checks inside the dump function to handle the case:

diff --git a/libde265/pps.cc b/libde265/pps.cc
index de3bcda1..a5abd9b3 100644
--- a/libde265/pps.cc
+++ b/libde265/pps.cc
@@ -903,14 +903,22 @@ void pic_parameter_set::dump(int fd) const
     LOG1("uniform_spacing_flag: %d\n", uniform_spacing_flag);

     LOG0("tile column boundaries: ");
-    for (int i=0;i<=num_tile_columns;i++) {
-      LOG1("*%d ",colBd[i]);
+    if (num_tile_columns+1 > DE265_MAX_TILE_COLUMNS) {
+      LOG0("Number of tile columns is invalid");
+    } else {
+      for (int i=0;i<=num_tile_columns;i++) {
+        LOG1("*%d ",colBd[i]);
+      }
     }
     LOG0("*\n");

     LOG0("tile row boundaries: ");
-    for (int i=0;i<=num_tile_rows;i++) {
-      LOG1("*%d ",rowBd[i]);
+    if (num_tile_rows+1 > DE265_MAX_TILE_ROWS) {
+      LOG0("Number of tile rows is invalid");
+    } else {
+      for (int i=0;i<=num_tile_rows;i++) {
+        LOG1("*%d ",rowBd[i]);
+      }
     }
     LOG0("*\n");

Other notes

The same issue occurs with num_tile_rows.

farindk commented 1 year ago

Thank you. I have fixed this in a way similar to your first patch, just reordering it a bit to match the code for reading vps, sps, and slice headers.

litios commented 1 year ago

Hi, thanks for the quick response and fix! It would be highly appreciated if you could request a CVE for this issue (or let me know if you want me to handle it).

Template for Mitre

Vulnerability type: Other or unknown
Other vulnerability type CWE-125
Vendor: strukturag
Product: libde265
Version: 1.0.12
Attack type: Context-dependent
Impact: Denial of Service, Information Disclosure
Affected component(s): pic_parameter_set::dump due to an incorrect value of num_tile_columns or num_tile_rows.
Attack vector(s): To exploit the vulnerability, someone needs to open an H.265 file and enable header dump.
Description: There is a segmentation fault caused by a buffer over-read in libde265. An attacker could leak information or cause a crash when decoding a specially crafted H.265 file if the dump headers option is enabled.
Discoverer(s)/Credits: David Fernández González (Canonical, Security team)
References: https://github.com/strukturag/libde265/issues/418, https://github.com/strukturag/libde265/commit/63b596c915977f038eafd7647d1db25488a8c133

Feel free to change or modify anything you feel is wrong! Thanks.

farindk commented 1 year ago

Thank you. I've sent the CVE request. Will update this once the number is published.

farindk commented 11 months ago

CVE-2023-43887 was assigned to this.

alteholz commented 10 months ago

Where did you send the request for this CVE? According to [1] the CVE is reserved, but no information are available yet.

[1] https://www.cve.org/CVERecord?id=CVE-2023-43887 [2] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-43887

farindk commented 10 months ago

According to [1] the CVE is reserved, but no information are available yet.

It is online now.

Crispy-fried-chicken commented 2 weeks ago

Maybe v1.0.6~v1.0.10 is also affected?

strukturag / libde265