diff options
author | William Wilgus <me.theuser@yahoo.com> | 2018-12-13 10:39:49 -0600 |
---|---|---|
committer | William Wilgus <me.theuser@yahoo.com> | 2018-12-14 01:28:17 -0600 |
commit | 3f110daf3032187c052a6e3c1b05d01d1a4582d0 (patch) | |
tree | d2fcb43e9010dd7f5b414b68d23448dfcccd8513 /apps/tree.c | |
parent | ce0b31d87db3c4c1c1bfb535c50770d33e9c4aaf (diff) | |
download | rockbox-3f110daf3032187c052a6e3c1b05d01d1a4582d0.tar.gz rockbox-3f110daf3032187c052a6e3c1b05d01d1a4582d0.zip |
Fix tree.c->tree_get_entry_at() buffer overflow
I observed a crash on buflib>move_block
after dumping ram I noticed that the buffer for filetypes was being corrupted
tree_get_entry_at returns a entry from the buflib 'tree entry' buffer
filetree.c->ft_load writes data to this buffer before checking if it has
reached the last entry resulting in buffer overflow that overwrites the
next entry in the buffer ['filetypes']
Patch checks that the index passed to tree_get_entry_at() is in range
otherwise it returns NULL
Added checks + panic in other functions using tree_get_entry_at()
Fixed tree_lock_cache() calls in playlist and filetree
Change-Id: Ibf9e65652b4e00445e8e509629aebbcddffcfd4d
Diffstat (limited to 'apps/tree.c')
-rw-r--r-- | apps/tree.c | 69 |
1 files changed, 47 insertions, 22 deletions
diff --git a/apps/tree.c b/apps/tree.c index 16e0f988dc..b5c9ddc11d 100644 --- a/apps/tree.c +++ b/apps/tree.c | |||
@@ -22,6 +22,7 @@ | |||
22 | #include <stdlib.h> | 22 | #include <stdlib.h> |
23 | #include <stdbool.h> | 23 | #include <stdbool.h> |
24 | #include "string-extra.h" | 24 | #include "string-extra.h" |
25 | #include "panic.h" | ||
25 | 26 | ||
26 | #include "applimits.h" | 27 | #include "applimits.h" |
27 | #include "dir.h" | 28 | #include "dir.h" |
@@ -111,11 +112,12 @@ struct entry* tree_get_entries(struct tree_context *t) | |||
111 | 112 | ||
112 | struct entry* tree_get_entry_at(struct tree_context *t, int index) | 113 | struct entry* tree_get_entry_at(struct tree_context *t, int index) |
113 | { | 114 | { |
115 | if(index < 0 || index >= t->cache.max_entries) | ||
116 | return NULL; /* no entry */ | ||
114 | struct entry* entries = tree_get_entries(t); | 117 | struct entry* entries = tree_get_entries(t); |
115 | return &entries[index]; | 118 | return &entries[index]; |
116 | } | 119 | } |
117 | 120 | ||
118 | |||
119 | static const char* tree_get_filename(int selected_item, void *data, | 121 | static const char* tree_get_filename(int selected_item, void *data, |
120 | char *buffer, size_t buffer_len) | 122 | char *buffer, size_t buffer_len) |
121 | { | 123 | { |
@@ -133,9 +135,11 @@ static const char* tree_get_filename(int selected_item, void *data, | |||
133 | else | 135 | else |
134 | #endif | 136 | #endif |
135 | { | 137 | { |
136 | struct entry* e = tree_get_entry_at(local_tc, selected_item); | 138 | struct entry *entry = tree_get_entry_at(local_tc, selected_item); |
137 | name = e->name; | 139 | if (!entry) |
138 | attr = e->attr; | 140 | panicf("Invalid tree entry"); |
141 | name = entry->name; | ||
142 | attr = entry->attr; | ||
139 | } | 143 | } |
140 | 144 | ||
141 | if(!(attr & ATTR_DIRECTORY)) | 145 | if(!(attr & ATTR_DIRECTORY)) |
@@ -175,8 +179,11 @@ static int tree_get_filecolor(int selected_item, void * data) | |||
175 | if (*tc.dirfilter == SHOW_ID3DB) | 179 | if (*tc.dirfilter == SHOW_ID3DB) |
176 | return -1; | 180 | return -1; |
177 | struct tree_context * local_tc=(struct tree_context *)data; | 181 | struct tree_context * local_tc=(struct tree_context *)data; |
178 | struct entry* e = tree_get_entry_at(local_tc, selected_item); | 182 | struct entry *entry = tree_get_entry_at(local_tc, selected_item); |
179 | return filetype_get_color(e->name, e->attr); | 183 | if (!entry) |
184 | panicf("Invalid tree entry"); | ||
185 | |||
186 | return filetype_get_color(entry->name, entry->attr); | ||
180 | } | 187 | } |
181 | #endif | 188 | #endif |
182 | 189 | ||
@@ -191,8 +198,11 @@ static enum themable_icons tree_get_fileicon(int selected_item, void * data) | |||
191 | else | 198 | else |
192 | #endif | 199 | #endif |
193 | { | 200 | { |
194 | struct entry* e = tree_get_entry_at(local_tc, selected_item); | 201 | struct entry *entry = tree_get_entry_at(local_tc, selected_item); |
195 | return filetype_get_icon(e->attr); | 202 | if (!entry) |
203 | panicf("Invalid tree entry"); | ||
204 | |||
205 | return filetype_get_icon(entry->attr); | ||
196 | } | 206 | } |
197 | } | 207 | } |
198 | 208 | ||
@@ -213,9 +223,12 @@ static int tree_voice_cb(int selected_item, void * data) | |||
213 | else | 223 | else |
214 | #endif | 224 | #endif |
215 | { | 225 | { |
216 | struct entry* e = tree_get_entry_at(local_tc, selected_item); | 226 | struct entry *entry = tree_get_entry_at(local_tc, selected_item); |
217 | name = e->name; | 227 | if (!entry) |
218 | attr = e->attr; | 228 | panicf("Invalid tree entry"); |
229 | |||
230 | name = entry->name; | ||
231 | attr = entry->attr; | ||
219 | } | 232 | } |
220 | bool is_dir = (attr & ATTR_DIRECTORY); | 233 | bool is_dir = (attr & ATTR_DIRECTORY); |
221 | bool did_clip = false; | 234 | bool did_clip = false; |
@@ -318,17 +331,22 @@ struct tree_context* tree_get_context(void) | |||
318 | */ | 331 | */ |
319 | static int tree_get_file_position(char * filename) | 332 | static int tree_get_file_position(char * filename) |
320 | { | 333 | { |
321 | int i; | 334 | int i, ret = -1;/* no file match, return undefined */ |
322 | struct entry* e; | 335 | |
336 | tree_lock_cache(&tc); | ||
337 | struct entry *entries = tree_get_entries(&tc); | ||
323 | 338 | ||
324 | /* use lastfile to determine the selected item (default=0) */ | 339 | /* use lastfile to determine the selected item (default=0) */ |
325 | for (i=0; i < tc.filesindir; i++) | 340 | for (i=0; i < tc.filesindir; i++) |
326 | { | 341 | { |
327 | e = tree_get_entry_at(&tc, i); | 342 | if (!strcasecmp(entries[i].name, filename)) |
328 | if (!strcasecmp(e->name, filename)) | 343 | { |
329 | return(i); | 344 | ret = i; |
345 | break; | ||
346 | } | ||
330 | } | 347 | } |
331 | return(-1);/* no file can match, returns undefined */ | 348 | tree_unlock_cache(&tc); |
349 | return(ret); | ||
332 | } | 350 | } |
333 | 351 | ||
334 | /* | 352 | /* |
@@ -527,14 +545,14 @@ char* get_current_file(char* buffer, size_t buffer_len) | |||
527 | return NULL; | 545 | return NULL; |
528 | #endif | 546 | #endif |
529 | 547 | ||
530 | struct entry* e = tree_get_entry_at(&tc, tc.selected_item); | 548 | struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); |
531 | if (getcwd(buffer, buffer_len)) | 549 | if (entry && getcwd(buffer, buffer_len)) |
532 | { | 550 | { |
533 | if (tc.dirlength) | 551 | if (tc.dirlength) |
534 | { | 552 | { |
535 | if (buffer[strlen(buffer)-1] != '/') | 553 | if (buffer[strlen(buffer)-1] != '/') |
536 | strlcat(buffer, "/", buffer_len); | 554 | strlcat(buffer, "/", buffer_len); |
537 | if (strlcat(buffer, e->name, buffer_len) >= buffer_len) | 555 | if (strlcat(buffer, entry->name, buffer_len) >= buffer_len) |
538 | return NULL; | 556 | return NULL; |
539 | } | 557 | } |
540 | return buffer; | 558 | return buffer; |
@@ -670,7 +688,11 @@ static int dirbrowse(void) | |||
670 | if ( numentries == 0 ) | 688 | if ( numentries == 0 ) |
671 | break; | 689 | break; |
672 | 690 | ||
673 | short attr = tree_get_entry_at(&tc, tc.selected_item)->attr; | 691 | struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); |
692 | if (!entry) | ||
693 | panicf("Invalid tree entry"); | ||
694 | |||
695 | short attr = entry->attr; | ||
674 | if ((tc.browse->flags & BROWSE_SELECTONLY) && | 696 | if ((tc.browse->flags & BROWSE_SELECTONLY) && |
675 | !(attr & ATTR_DIRECTORY)) | 697 | !(attr & ATTR_DIRECTORY)) |
676 | { | 698 | { |
@@ -798,6 +820,9 @@ static int dirbrowse(void) | |||
798 | #endif | 820 | #endif |
799 | { | 821 | { |
800 | struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); | 822 | struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); |
823 | if (!entry) | ||
824 | panicf("Invalid tree entry"); | ||
825 | |||
801 | attr = entry->attr; | 826 | attr = entry->attr; |
802 | 827 | ||
803 | if (currdir[1]) /* Not in / */ | 828 | if (currdir[1]) /* Not in / */ |
@@ -1017,7 +1042,7 @@ static int move_callback(int handle, void* current, void* new) | |||
1017 | if (cache->lock_count > 0) | 1042 | if (cache->lock_count > 0) |
1018 | return BUFLIB_CB_CANNOT_MOVE; | 1043 | return BUFLIB_CB_CANNOT_MOVE; |
1019 | 1044 | ||
1020 | size_t diff = new - current; | 1045 | ptrdiff_t diff = (int32_t *) new - (int32_t *) current; |
1021 | /* FIX_PTR makes sure to not accidentally update static allocations */ | 1046 | /* FIX_PTR makes sure to not accidentally update static allocations */ |
1022 | #define FIX_PTR(x) \ | 1047 | #define FIX_PTR(x) \ |
1023 | { if ((void*)x >= current && (void*)x < (current+cache->name_buffer_size)) x+= diff; } | 1048 | { if ((void*)x >= current && (void*)x < (current+cache->name_buffer_size)) x+= diff; } |