Skip to content

Commit

Permalink
Fix crash caused by Use after free
Browse files Browse the repository at this point in the history 
ext_image is released after the for loop, do not create it, write
directly to the ext_artwork array.

b/329330347
  • Loading branch information
Colin Liang committed Mar 26, 2024
1 parent c65b33a commit 31b5462
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 53 deletions.
98 changes: 48 additions & 50 deletions cobalt/media_session/media_session_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <cmath>
#include <memory>
#include <string>
#include <vector>

#include "base/logging.h"
#include "cobalt/media_session/media_image.h"
Expand Down Expand Up @@ -297,60 +298,57 @@ void MediaSessionClient::UpdateMediaSessionState() {

void MediaSessionClient::OnMediaSessionStateChanged(
const MediaSessionState& session_state) {
if (extension_ && extension_->version >= 1) {
CobaltExtensionMediaSessionState ext_state;
size_t artwork_size = 0;
if (session_state.has_metadata() &&
session_state.metadata().value().has_artwork()) {
artwork_size = session_state.metadata().value().artwork().size();
}
std::unique_ptr<CobaltExtensionMediaImage[]> ext_artwork =
std::unique_ptr<CobaltExtensionMediaImage[]>(
new CobaltExtensionMediaImage[artwork_size]);

ext_state.duration = session_state.duration();
ext_state.actual_playback_rate = session_state.actual_playback_rate();
ext_state.current_playback_position =
session_state.current_playback_position();
ext_state.has_position_state = session_state.has_position_state();
ext_state.actual_playback_state =
ConvertPlaybackState(session_state.actual_playback_state());
ConvertMediaSessionActions(session_state.available_actions(),
ext_state.available_actions);
std::string album = "";
std::string artist = "";
std::string title = "";

if (session_state.has_metadata()) {
const MediaMetadataInit& metadata = session_state.metadata().value();
album = metadata.album();
artist = metadata.artist();
title = metadata.title();
if (artwork_size > 0) {
const MediaImageSequence& artwork(metadata.artwork());
for (MediaImageSequence::size_type i = 0; i < artwork_size; i++) {
const MediaImage& media_image(artwork.at(i));
CobaltExtensionMediaImage ext_image;
ext_image.src = media_image.src().c_str();
if (ext_image.src == nullptr) {
// src() is required, but Cobalt IDL parser doesn't enforce it.
// See cobalt/media_session/media_image.idl for more info.
// https://wicg.github.io/mediasession/#dictdef-mediaimage
LOG(ERROR) << "Required src string for MediaImage is missing.";
}
ext_image.size = media_image.sizes().c_str();
ext_image.type = media_image.type().c_str();
ext_artwork[i] = ext_image;
if (!extension_ || extension_->version < 1) {
return;
}

CobaltExtensionMediaSessionState ext_state;
size_t artwork_size = 0;
if (session_state.has_metadata() &&
session_state.metadata().value().has_artwork()) {
artwork_size = session_state.metadata().value().artwork().size();
}
ext_state.duration = session_state.duration();
ext_state.actual_playback_rate = session_state.actual_playback_rate();
ext_state.current_playback_position =
session_state.current_playback_position();
ext_state.has_position_state = session_state.has_position_state();
ext_state.actual_playback_state =
ConvertPlaybackState(session_state.actual_playback_state());
ConvertMediaSessionActions(session_state.available_actions(),
ext_state.available_actions);
std::string album = "";
std::string artist = "";
std::string title = "";
std::vector<CobaltExtensionMediaImage> ext_artwork(artwork_size);

if (session_state.has_metadata()) {
const MediaMetadataInit& metadata = session_state.metadata().value();
album = metadata.album();
artist = metadata.artist();
title = metadata.title();
if (artwork_size > 0) {
const MediaImageSequence& artwork = metadata.artwork();
for (MediaImageSequence::size_type i = 0; i < artwork_size; i++) {
const MediaImage& media_image = artwork.at(i);
ext_artwork[i].src = media_image.src().c_str();
if (ext_artwork[i].src == nullptr) {
// src() is required, but Cobalt IDL parser doesn't enforce it.
// See cobalt/media_session/media_image.idl for more info.
// https://wicg.github.io/mediasession/#dictdef-mediaimage
LOG(ERROR) << "Required src string for MediaImage is missing.";
}
ext_artwork[i].size = media_image.sizes().c_str();
ext_artwork[i].type = media_image.type().c_str();
}
}
CobaltExtensionMediaMetadata ext_metadata = {
album.c_str(), artist.c_str(), title.c_str(), ext_artwork.get(),
artwork_size};
ext_state.metadata = &ext_metadata;

extension_->OnMediaSessionStateChanged(ext_state);
}
CobaltExtensionMediaMetadata ext_metadata = {
album.c_str(), artist.c_str(), title.c_str(), ext_artwork.data(),
artwork_size};
ext_state.metadata = &ext_metadata;

extension_->OnMediaSessionStateChanged(ext_state);
}

// static
Expand Down
6 changes: 3 additions & 3 deletions starboard/android/shared/android_media_session_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,15 @@ void OnMediaSessionStateChanged(
ScopedLocalJavaRef<jobjectArray> j_artwork;

if (session_state.metadata != NULL) {
CobaltExtensionMediaMetadata* media_metadata(session_state.metadata);
const CobaltExtensionMediaMetadata* media_metadata = session_state.metadata;

j_title.Reset(env->NewStringStandardUTFOrAbort(media_metadata->title));
j_artist.Reset(env->NewStringStandardUTFOrAbort(media_metadata->artist));
j_album.Reset(env->NewStringStandardUTFOrAbort(media_metadata->album));

size_t artwork_count = media_metadata->artwork_count;
if (artwork_count > 0) {
CobaltExtensionMediaImage* artwork(media_metadata->artwork);
const CobaltExtensionMediaImage* artwork = media_metadata->artwork;
ScopedLocalJavaRef<jclass> media_image_class(
env->FindClassExtOrAbort("dev/cobalt/media/MediaImage"));
jmethodID media_image_constructor = env->GetMethodID(
Expand All @@ -210,7 +210,7 @@ void OnMediaSessionStateChanged(
ScopedLocalJavaRef<jstring> j_sizes;
ScopedLocalJavaRef<jstring> j_type;
for (size_t i = 0; i < artwork_count; i++) {
const CobaltExtensionMediaImage& media_image(artwork[i]);
const CobaltExtensionMediaImage& media_image = artwork[i];
j_src.Reset(env->NewStringStandardUTFOrAbort(media_image.src));
j_sizes.Reset(env->NewStringStandardUTFOrAbort(media_image.size));
j_type.Reset(env->NewStringStandardUTFOrAbort(media_image.type));
Expand Down

0 comments on commit 31b5462

Please sign in to comment.