[video_player_avplay] Fix picture subtitle display issue#964
Conversation
There was a problem hiding this comment.
Code Review
This pull request enables picture subtitle display for DASH streams by removing conditional compilation flags in plus_player.cc. However, the newly exposed code contains unsafe numeric operations on untrusted input, which could lead to integer overflows, undefined behavior, and out-of-bounds reads. This poses a significant security risk, potentially causing a denial-of-service (crash) or memory corruption when processing malicious subtitle data. The package version has also been updated to 0.8.3 in pubspec.yaml, CHANGELOG.md, and README.md.
| } | ||
|
|
||
| if (type == plusplayer::SubtitleType::kPicture) { | ||
| #ifdef PICTURE_SUBTITLE_SUPPORT |
There was a problem hiding this comment.
The removal of #ifdef PICTURE_SUBTITLE_SUPPORT enables code that processes picture subtitles. This code contains several unsafe numeric operations on untrusted input (picture_width and picture_height from the player).
- Undefined Behavior (UB) in
channelscalculation (line 1330):int channels = size / area;. Ifareais very small (e.g., due to underflow ofpicture_width * picture_height),size / areacan result inInf. CastingInftointis undefined behavior. - Integer Overflow in
stride_in_bytes(line 1335):int stride_in_bytes = static_cast<int>(picture_width) * channels;. Ifpicture_widthis large (e.g.,2e9) andchannelsis2, the product overflowsint. - Undefined Behavior in
static_cast<int>(picture_width)(line 1335, 1339): Ifpicture_widthexceedsINT_MAX, the cast is undefined behavior.
These issues can lead to an incorrect stride_in_bytes or dimensions being passed to stbi_write_png_to_mem (line 1337), which can cause an out-of-bounds read from the data buffer, potentially leading to a crash (DoS) or memory corruption.
Main changes: