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

[litios]

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: 6fc550f (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]

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]

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: Buffer over-read causes segmentation fault in pic_parameter_set::dump #418, 63b596c

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

[farindk]

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

[farindk]

CVE-2023-43887 was assigned to this.

[alteholz]

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]

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

It is online now.

[Crispy-fried-chicken]

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