diff options
| author | Christian Soffke <christian.soffke@gmail.com> | 2024-11-23 14:59:03 +0100 |
|---|---|---|
| committer | Christian Soffke <christian.soffke@gmail.com> | 2024-11-30 15:45:40 +0100 |
| commit | 0469a95bee248339e2eb5770b3161500f2712491 (patch) | |
| tree | cb3683be381f37cf1674ae966053d1844a2cd28b | |
| parent | c6fb789b3baddd0ea550c3aeac5bd3e2574d8002 (diff) | |
| download | rockbox-0469a95bee.tar.gz rockbox-0469a95bee.zip | |
Playlist Viewer: Mitigate FS#13518 (stack overflow)
Playlist Viewer hangs indefinitely on tracks with long Comment fields
Commit 2156d98 (metadata: mp3: Improve support for long tags) has
allowed the length of parsed tags to exceed ID3V2_MAX_ITEM_SIZE.
Overly long tags may blow the stack due to a stack-allocated buffer
in the metadata parsing routine, that can reach 3 times the size of
the id3v2 and id3v1 buffers combined.
This patch moves the mp3entry struct used by the Playlist Viewer
off the stack and onto the plugin buffer instead, which helps prevent
a stack overflow in cases such as the example posted in FS#13518.
Future patches to either constrain ID3V2_BUF_SIZE to ~1120 bytes or
to lower the stack usage of metadata parsing will still be required
to fully address the issue.
Change-Id: I2f436195bc8df4d785b1880f6ab167783a162020
| -rw-r--r-- | apps/playlist_viewer.c | 21 |
1 files changed, 14 insertions, 7 deletions
diff --git a/apps/playlist_viewer.c b/apps/playlist_viewer.c index c3c6f6fa29..2b457ba1c9 100644 --- a/apps/playlist_viewer.c +++ b/apps/playlist_viewer.c @@ -115,6 +115,7 @@ struct playlist_viewer { int moving_playlist_index; /* Playlist-relative index (as opposed to viewer-relative index) of moving track */ struct playlist_buffer buffer; + struct mp3entry *id3; }; struct playlist_search_data @@ -385,10 +386,16 @@ static bool playlist_viewer_init(struct playlist_viewer * viewer, return false; } + size_t id3_size = sizeof(*viewer->id3); + buffer = plugin_get_buffer(&buffer_size); - if (!buffer || buffer_size <= MAX_PATH) + if (!buffer || buffer_size <= MAX_PATH + id3_size) return false; + viewer->id3 = (void *) buffer; + buffer += id3_size; + buffer_size -= id3_size; + if (!filename) { viewer->playlist = NULL; @@ -491,16 +498,15 @@ static void format_line(struct playlist_entry* track, char* str, )) { track->attr |= PLAYLIST_ATTR_RETRIEVE_ID3_ATTEMPTED; - struct mp3entry id3; bool retrieve_success = retrieve_id3_tags(track->index, track->name, - &id3, METADATA_EXCLUDE_ID3_PATH); + viewer.id3, METADATA_EXCLUDE_ID3_PATH); if (retrieve_success) { if (!id3viewc) { id3viewc = track->name + strlen(track->name) + 1; } - struct mp3entry * pid3 = &id3; + struct mp3entry * pid3 = viewer.id3; id3viewc[0] = '\0'; if (global_settings.playlist_viewer_track_display == PLAYLIST_VIEWER_ENTRY_SHOW_ID3_TITLE_AND_ALBUM) @@ -573,11 +579,12 @@ static void format_line(struct playlist_entry* track, char* str, static enum pv_onplay_result show_track_info(const struct playlist_entry *current_track) { - struct mp3entry id3; - bool id3_retrieval_successful = retrieve_id3_tags(current_track->index, current_track->name, &id3, 0); + bool id3_retrieval_successful = retrieve_id3_tags(current_track->index, + current_track->name, + viewer.id3, 0); return id3_retrieval_successful && - browse_id3_ex(&id3, viewer.playlist, current_track->display_index, + browse_id3_ex(viewer.id3, viewer.playlist, current_track->display_index, viewer.num_tracks, NULL, 1) ? PV_ONPLAY_USB : PV_ONPLAY_UNCHANGED; } |