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 @@ #include <stdlib.h> #include <stdbool.h> #include "string-extra.h" +#include "panic.h" #include "applimits.h" #include "dir.h" @@ -111,11 +112,12 @@ struct entry* tree_get_entries(struct tree_context *t) struct entry* tree_get_entry_at(struct tree_context *t, int index) { + if(index < 0 || index >= t->cache.max_entries) + return NULL; /* no entry */ struct entry* entries = tree_get_entries(t); return &entries[index]; } - static const char* tree_get_filename(int selected_item, void *data, char *buffer, size_t buffer_len) { @@ -133,9 +135,11 @@ static const char* tree_get_filename(int selected_item, void *data, else #endif { - struct entry* e = tree_get_entry_at(local_tc, selected_item); - name = e->name; - attr = e->attr; + struct entry *entry = tree_get_entry_at(local_tc, selected_item); + if (!entry) + panicf("Invalid tree entry"); + name = entry->name; + attr = entry->attr; } if(!(attr & ATTR_DIRECTORY)) @@ -175,8 +179,11 @@ static int tree_get_filecolor(int selected_item, void * data) if (*tc.dirfilter == SHOW_ID3DB) return -1; struct tree_context * local_tc=(struct tree_context *)data; - struct entry* e = tree_get_entry_at(local_tc, selected_item); - return filetype_get_color(e->name, e->attr); + struct entry *entry = tree_get_entry_at(local_tc, selected_item); + if (!entry) + panicf("Invalid tree entry"); + + return filetype_get_color(entry->name, entry->attr); } #endif @@ -191,8 +198,11 @@ static enum themable_icons tree_get_fileicon(int selected_item, void * data) else #endif { - struct entry* e = tree_get_entry_at(local_tc, selected_item); - return filetype_get_icon(e->attr); + struct entry *entry = tree_get_entry_at(local_tc, selected_item); + if (!entry) + panicf("Invalid tree entry"); + + return filetype_get_icon(entry->attr); } } @@ -213,9 +223,12 @@ static int tree_voice_cb(int selected_item, void * data) else #endif { - struct entry* e = tree_get_entry_at(local_tc, selected_item); - name = e->name; - attr = e->attr; + struct entry *entry = tree_get_entry_at(local_tc, selected_item); + if (!entry) + panicf("Invalid tree entry"); + + name = entry->name; + attr = entry->attr; } bool is_dir = (attr & ATTR_DIRECTORY); bool did_clip = false; @@ -318,17 +331,22 @@ struct tree_context* tree_get_context(void) */ static int tree_get_file_position(char * filename) { - int i; - struct entry* e; + int i, ret = -1;/* no file match, return undefined */ + + tree_lock_cache(&tc); + struct entry *entries = tree_get_entries(&tc); /* use lastfile to determine the selected item (default=0) */ for (i=0; i < tc.filesindir; i++) { - e = tree_get_entry_at(&tc, i); - if (!strcasecmp(e->name, filename)) - return(i); + if (!strcasecmp(entries[i].name, filename)) + { + ret = i; + break; + } } - return(-1);/* no file can match, returns undefined */ + tree_unlock_cache(&tc); + return(ret); } /* @@ -527,14 +545,14 @@ char* get_current_file(char* buffer, size_t buffer_len) return NULL; #endif - struct entry* e = tree_get_entry_at(&tc, tc.selected_item); - if (getcwd(buffer, buffer_len)) + struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); + if (entry && getcwd(buffer, buffer_len)) { if (tc.dirlength) { if (buffer[strlen(buffer)-1] != '/') strlcat(buffer, "/", buffer_len); - if (strlcat(buffer, e->name, buffer_len) >= buffer_len) + if (strlcat(buffer, entry->name, buffer_len) >= buffer_len) return NULL; } return buffer; @@ -670,7 +688,11 @@ static int dirbrowse(void) if ( numentries == 0 ) break; - short attr = tree_get_entry_at(&tc, tc.selected_item)->attr; + struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); + if (!entry) + panicf("Invalid tree entry"); + + short attr = entry->attr; if ((tc.browse->flags & BROWSE_SELECTONLY) && !(attr & ATTR_DIRECTORY)) { @@ -798,6 +820,9 @@ static int dirbrowse(void) #endif { struct entry *entry = tree_get_entry_at(&tc, tc.selected_item); + if (!entry) + panicf("Invalid tree entry"); + attr = entry->attr; if (currdir[1]) /* Not in / */ @@ -1017,7 +1042,7 @@ static int move_callback(int handle, void* current, void* new) if (cache->lock_count > 0) return BUFLIB_CB_CANNOT_MOVE; - size_t diff = new - current; + ptrdiff_t diff = (int32_t *) new - (int32_t *) current; /* FIX_PTR makes sure to not accidentally update static allocations */ #define FIX_PTR(x) \ { if ((void*)x >= current && (void*)x < (current+cache->name_buffer_size)) x+= diff; } |