diff options
author | Christian Soffke <christian.soffke@gmail.com> | 2022-01-01 14:53:44 +0100 |
---|---|---|
committer | William Wilgus <me.theuser@yahoo.com> | 2022-01-04 18:02:12 -0500 |
commit | dded97be3413efd8d816bf3cffa2aeb9b323216f (patch) | |
tree | f185033532d75bdf28383642b1d5f00ef2e477e0 /apps/plugins | |
parent | e3b8b7fa80320f0c8bbc84d4364ea83b5991db20 (diff) | |
download | rockbox-dded97be3413efd8d816bf3cffa2aeb9b323216f.tar.gz rockbox-dded97be3413efd8d816bf3cffa2aeb9b323216f.zip |
PictureFlow: Fix buffer overflow
create_track_index appears to have relied on
buflib_buffer_out returning a certain amount
of space without checking that it was actually
available. In at least one test case, as little as
16 bytes were returned, leading to a buffer
overflow and later a segfault.
Change-Id: Ic0783f3cd5bf015803b7ce90537ba38ab3434bea
Diffstat (limited to 'apps/plugins')
-rw-r--r-- | apps/plugins/pictureflow/pictureflow.c | 227 |
1 files changed, 126 insertions, 101 deletions
diff --git a/apps/plugins/pictureflow/pictureflow.c b/apps/plugins/pictureflow/pictureflow.c index d2b9ae5190..7801377bda 100644 --- a/apps/plugins/pictureflow/pictureflow.c +++ b/apps/plugins/pictureflow/pictureflow.c | |||
@@ -330,6 +330,7 @@ struct pf_track_t { | |||
330 | int list_y; | 330 | int list_y; |
331 | int list_h; | 331 | int list_h; |
332 | size_t borrowed; | 332 | size_t borrowed; |
333 | size_t used; | ||
333 | struct track_data *index; | 334 | struct track_data *index; |
334 | char *names; | 335 | char *names; |
335 | }; | 336 | }; |
@@ -1665,13 +1666,109 @@ static int compare_tracks (const void *a_v, const void *b_v) | |||
1665 | return (int)(a - b); | 1666 | return (int)(a - b); |
1666 | } | 1667 | } |
1667 | 1668 | ||
1669 | |||
1670 | |||
1671 | static bool track_buffer_avail(size_t needed) | ||
1672 | { | ||
1673 | size_t total_out = 0; | ||
1674 | size_t out = 0; | ||
1675 | if (pf_tracks.borrowed == 0 && pf_tracks.used == 0) | ||
1676 | { | ||
1677 | pf_tracks.names = rb->buflib_buffer_out(&buf_ctx, &out); | ||
1678 | pf_tracks.borrowed = out; | ||
1679 | } | ||
1680 | |||
1681 | if (needed <= pf_tracks.borrowed - pf_tracks.used) | ||
1682 | return true; | ||
1683 | |||
1684 | while (needed > (pf_tracks.borrowed + total_out) - pf_tracks.used) | ||
1685 | { | ||
1686 | if (!free_slide_prio(0)) | ||
1687 | break; | ||
1688 | out = 0; | ||
1689 | rb->buflib_buffer_out(&buf_ctx, &out); | ||
1690 | total_out += out; | ||
1691 | } | ||
1692 | pf_tracks.borrowed += total_out; | ||
1693 | |||
1694 | // have to move already stored track_data structs | ||
1695 | if (pf_tracks.count) | ||
1696 | { | ||
1697 | struct track_data *new_tracks = (struct track_data *)(total_out + (uintptr_t)pf_tracks.index); | ||
1698 | unsigned int bytes = pf_tracks.count * sizeof(struct track_data); | ||
1699 | rb->memmove(new_tracks, pf_tracks.index, bytes); | ||
1700 | } | ||
1701 | |||
1702 | if (needed > pf_tracks.borrowed - pf_tracks.used) | ||
1703 | return false; | ||
1704 | |||
1705 | return true; | ||
1706 | } | ||
1707 | |||
1708 | |||
1709 | static int pf_tcs_retrieve_track_title(int string_index, int disc_num, int track_num) | ||
1710 | { | ||
1711 | char file_name[MAX_PATH]; | ||
1712 | char *track_title = NULL; | ||
1713 | int str_len; | ||
1714 | |||
1715 | if (rb->strcmp(UNTAGGED, tcs.result) == 0) | ||
1716 | { | ||
1717 | /* show filename instead of <untaggged> */ | ||
1718 | if (!rb->tagcache_retrieve(&tcs, tcs.idx_id, tag_filename, | ||
1719 | file_name, MAX_PATH)) | ||
1720 | return 0; | ||
1721 | track_title = file_name; | ||
1722 | if (track_title) | ||
1723 | { | ||
1724 | /* if filename remove the '/' */ | ||
1725 | track_title = rb->strrchr(track_title, PATH_SEPCH); | ||
1726 | if (track_title) | ||
1727 | track_title++; | ||
1728 | } | ||
1729 | } | ||
1730 | |||
1731 | if (!track_title) | ||
1732 | track_title = tcs.result; | ||
1733 | |||
1734 | int max_len = rb->strlen(track_title) + 10; | ||
1735 | if (!track_buffer_avail(max_len)) | ||
1736 | return 0; | ||
1737 | |||
1738 | if (track_num > 0) | ||
1739 | { | ||
1740 | if (disc_num > 0) | ||
1741 | str_len = rb->snprintf(pf_tracks.names + string_index, max_len, | ||
1742 | "%d.%02d: %s", disc_num, track_num, track_title); | ||
1743 | else | ||
1744 | str_len = rb->snprintf(pf_tracks.names + string_index, max_len, | ||
1745 | "%d: %s", track_num, track_title); | ||
1746 | } | ||
1747 | else | ||
1748 | str_len = rb->snprintf(pf_tracks.names + string_index, max_len, | ||
1749 | "%s", track_title); | ||
1750 | return str_len; | ||
1751 | } | ||
1752 | |||
1753 | #if PF_PLAYBACK_CAPABLE | ||
1754 | static int pf_tcs_retrieve_file_name(int fn_idx) | ||
1755 | { | ||
1756 | if (!track_buffer_avail(MAX_PATH)) | ||
1757 | return 0; | ||
1758 | |||
1759 | rb->tagcache_retrieve(&tcs, tcs.idx_id, tag_filename, | ||
1760 | pf_tracks.names + fn_idx, MAX_PATH); | ||
1761 | |||
1762 | return rb->strlen(pf_tracks.names + fn_idx); | ||
1763 | } | ||
1764 | #endif | ||
1765 | |||
1668 | /** | 1766 | /** |
1669 | Create the track index of the given slide_index. | 1767 | Create the track index of the given slide_index. |
1670 | */ | 1768 | */ |
1671 | static void create_track_index(const int slide_index) | 1769 | static void create_track_index(const int slide_index) |
1672 | { | 1770 | { |
1673 | buf_ctx_lock(); | 1771 | buf_ctx_lock(); |
1674 | char temp[MAX_PATH + 1]; | ||
1675 | if ( slide_index == pf_tracks.cur_idx ) | 1772 | if ( slide_index == pf_tracks.cur_idx ) |
1676 | return; | 1773 | return; |
1677 | 1774 | ||
@@ -1682,124 +1779,48 @@ static void create_track_index(const int slide_index) | |||
1682 | pf_idx.album_index[slide_index].seek); | 1779 | pf_idx.album_index[slide_index].seek); |
1683 | 1780 | ||
1684 | if (pf_idx.album_index[slide_index].artist_idx >= 0) | 1781 | if (pf_idx.album_index[slide_index].artist_idx >= 0) |
1685 | { | ||
1686 | rb->tagcache_search_add_filter(&tcs, tag_albumartist, | 1782 | rb->tagcache_search_add_filter(&tcs, tag_albumartist, |
1687 | pf_idx.album_index[slide_index].artist_seek); | 1783 | pf_idx.album_index[slide_index].artist_seek); |
1688 | } | ||
1689 | |||
1690 | int string_index = 0, track_num; | ||
1691 | int disc_num; | ||
1692 | |||
1693 | char* result = NULL; | ||
1694 | size_t out = 0; | ||
1695 | 1784 | ||
1785 | int string_index = 0; | ||
1696 | pf_tracks.count = 0; | 1786 | pf_tracks.count = 0; |
1697 | pf_tracks.names = rb->buflib_buffer_out(&buf_ctx, &out); | 1787 | |
1698 | pf_tracks.borrowed += out; | ||
1699 | int avail = pf_tracks.borrowed; | ||
1700 | pf_tracks.index = (struct track_data*)(pf_tracks.names + pf_tracks.borrowed); | ||
1701 | while (rb->tagcache_get_next(&tcs)) | 1788 | while (rb->tagcache_get_next(&tcs)) |
1702 | { | 1789 | { |
1703 | result = NULL; | 1790 | int disc_num = rb->tagcache_get_numeric(&tcs, tag_discnumber); |
1704 | if (rb->strcmp(UNTAGGED, tcs.result) == 0) | 1791 | int track_num = rb->tagcache_get_numeric(&tcs, tag_tracknumber); |
1705 | { | 1792 | disc_num = disc_num > 0 ? disc_num : 0; |
1706 | /* show filename instead of <untaggged> */ | 1793 | track_num = track_num > 0 ? track_num : 0; |
1707 | if (!rb->tagcache_retrieve(&tcs, tcs.idx_id, tag_filename, | 1794 | int fn_idx = 1 + pf_tcs_retrieve_track_title(string_index, disc_num, track_num); |
1708 | temp, sizeof(temp) - 1)) | 1795 | if (fn_idx <= 1) |
1709 | { | ||
1710 | goto fail; | ||
1711 | } | ||
1712 | result = temp; | ||
1713 | } | ||
1714 | |||
1715 | int len = 0, fn_idx = 0; | ||
1716 | |||
1717 | avail -= sizeof(struct track_data); | ||
1718 | track_num = rb->tagcache_get_numeric(&tcs, tag_tracknumber); | ||
1719 | disc_num = rb->tagcache_get_numeric(&tcs, tag_discnumber); | ||
1720 | |||
1721 | if (result) | ||
1722 | { | ||
1723 | /* if filename remove the '/' */ | ||
1724 | result = rb->strrchr(result, PATH_SEPCH); | ||
1725 | if (result) | ||
1726 | result++; | ||
1727 | } | ||
1728 | |||
1729 | if (!result) | ||
1730 | result = tcs.result; | ||
1731 | |||
1732 | if (disc_num < 0) | ||
1733 | disc_num = 0; | ||
1734 | retry: | ||
1735 | if (track_num > 0) | ||
1736 | { | ||
1737 | if (disc_num) | ||
1738 | fn_idx = 1 + rb->snprintf(pf_tracks.names + string_index, avail, | ||
1739 | "%d.%02d: %s", disc_num, track_num, result); | ||
1740 | else | ||
1741 | fn_idx = 1 + rb->snprintf(pf_tracks.names + string_index, avail, | ||
1742 | "%d: %s", track_num, result); | ||
1743 | } | ||
1744 | else | ||
1745 | { | ||
1746 | track_num = 0; | ||
1747 | fn_idx = 1 + rb->snprintf(pf_tracks.names + string_index, avail, | ||
1748 | "%s", result); | ||
1749 | } | ||
1750 | if (fn_idx <= 0) | ||
1751 | goto fail; | 1796 | goto fail; |
1752 | #if PF_PLAYBACK_CAPABLE | 1797 | pf_tracks.used += fn_idx; |
1753 | int remain = avail - fn_idx; | ||
1754 | if (remain >= MAX_PATH) | ||
1755 | { /* retrieve filename for building the playlist */ | ||
1756 | rb->tagcache_retrieve(&tcs, tcs.idx_id, tag_filename, | ||
1757 | pf_tracks.names + string_index + fn_idx, remain); | ||
1758 | |||
1759 | len = fn_idx + rb->strlen(pf_tracks.names + string_index + fn_idx) + 1; | ||
1760 | /* make sure track name and file name are really split by a \0, else | ||
1761 | * get_track_name might fail */ | ||
1762 | *(pf_tracks.names + string_index + fn_idx -1) = '\0'; | ||
1763 | 1798 | ||
1764 | } | 1799 | #if PF_PLAYBACK_CAPABLE |
1765 | else /* request more buffer so that track and filename fit */ | 1800 | int fn_len = 1 + pf_tcs_retrieve_file_name(string_index + fn_idx); |
1766 | len = (avail - remain) + MAX_PATH; | 1801 | if (fn_len <= 1) |
1767 | #else | 1802 | goto fail; |
1768 | len = fn_idx; | 1803 | pf_tracks.used += fn_len; |
1769 | #endif | 1804 | #endif |
1770 | if (len > avail) | 1805 | if (!track_buffer_avail(sizeof(struct track_data))) |
1771 | { | 1806 | goto fail; |
1772 | while (len > avail) | ||
1773 | { | ||
1774 | if (!free_slide_prio(0)) | ||
1775 | goto fail; | ||
1776 | out = 0; | ||
1777 | rb->buflib_buffer_out(&buf_ctx, &out); | ||
1778 | avail += out; | ||
1779 | pf_tracks.borrowed += out; | ||
1780 | |||
1781 | struct track_data *new_tracks; | ||
1782 | new_tracks = (struct track_data *)(out + (uintptr_t)pf_tracks.index); | ||
1783 | |||
1784 | unsigned int bytes = pf_tracks.count * sizeof(struct track_data); | ||
1785 | if (pf_tracks.count) | ||
1786 | rb->memmove(new_tracks, pf_tracks.index, bytes); | ||
1787 | pf_tracks.index = new_tracks; | ||
1788 | } | ||
1789 | goto retry; | ||
1790 | } | ||
1791 | 1807 | ||
1792 | avail -= len; | 1808 | pf_tracks.used += sizeof(struct track_data); |
1793 | pf_tracks.index--; | 1809 | unsigned int arr_sz = (pf_tracks.count + 1) * sizeof(struct track_data); |
1810 | // Arrray descends from upper end of buflib-borrowed buffer. | ||
1811 | pf_tracks.index = (struct track_data*)(pf_tracks.names + pf_tracks.borrowed | ||
1812 | - arr_sz ); | ||
1794 | pf_tracks.index->sort = (disc_num << 24) + (track_num << 14); | 1813 | pf_tracks.index->sort = (disc_num << 24) + (track_num << 14); |
1795 | pf_tracks.index->sort += pf_tracks.count; | 1814 | pf_tracks.index->sort += pf_tracks.count; |
1796 | pf_tracks.index->name_idx = string_index; | 1815 | pf_tracks.index->name_idx = string_index; |
1797 | pf_tracks.index->seek = tcs.result_seek; | 1816 | pf_tracks.index->seek = tcs.result_seek; |
1798 | #if PF_PLAYBACK_CAPABLE | 1817 | #if PF_PLAYBACK_CAPABLE |
1799 | pf_tracks.index->filename_idx = fn_idx + string_index; | 1818 | pf_tracks.index->filename_idx = fn_idx + string_index; |
1819 | string_index += (fn_idx + fn_len); | ||
1820 | #else | ||
1821 | string_index += fn_idx; | ||
1800 | #endif | 1822 | #endif |
1801 | pf_tracks.count++; | 1823 | pf_tracks.count++; |
1802 | string_index += len; | ||
1803 | } | 1824 | } |
1804 | 1825 | ||
1805 | rb->tagcache_search_finish(&tcs); | 1826 | rb->tagcache_search_finish(&tcs); |
@@ -1824,6 +1845,7 @@ static inline void free_borrowed_tracks(void) | |||
1824 | { | 1845 | { |
1825 | rb->buflib_buffer_in(&buf_ctx, pf_tracks.borrowed); | 1846 | rb->buflib_buffer_in(&buf_ctx, pf_tracks.borrowed); |
1826 | pf_tracks.borrowed = 0; | 1847 | pf_tracks.borrowed = 0; |
1848 | pf_tracks.used = 0; | ||
1827 | pf_tracks.cur_idx = -1; | 1849 | pf_tracks.cur_idx = -1; |
1828 | buf_ctx_unlock(); | 1850 | buf_ctx_unlock(); |
1829 | } | 1851 | } |
@@ -3955,6 +3977,9 @@ static int pictureflow_main(const char* selected_file) | |||
3955 | pf_state = pf_idle; | 3977 | pf_state = pf_idle; |
3956 | 3978 | ||
3957 | pf_tracks.cur_idx = -1; | 3979 | pf_tracks.cur_idx = -1; |
3980 | pf_tracks.borrowed = 0; | ||
3981 | pf_tracks.used = 0; | ||
3982 | |||
3958 | extra_fade = 0; | 3983 | extra_fade = 0; |
3959 | slide_frame = 0; | 3984 | slide_frame = 0; |
3960 | step = 0; | 3985 | step = 0; |