Skip to content

Commit

Permalink
menu.c: Fix array out of bounds after mailbox deletion.
Browse files Browse the repository at this point in the history
Due to the possible reallocation of the mailboxes array
made possible by commit 9a61ece,
it is now possible for client->folders.n and client->num_mailboxes
to differ when free_folder_items is called. Index using the correct
variable to avoid this, and ensure that the display names are
cleaned up appropriately in both cases.
  • Loading branch information
InterLinked1 committed Oct 6, 2024
1 parent ac45fd4 commit 23759f8
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 1 deletion.
2 changes: 2 additions & 0 deletions evergreen.c
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ static int rerender_folder_pane(struct client *client, int selected_item)

/* Create menu options for each mailbox, then create the menu */
if (create_folder_items(client) || create_folder_menu(client)) {
client_error("Failed to create folder items or menu");
return -1;
}

Expand Down Expand Up @@ -585,6 +586,7 @@ static int render_message_pane(struct client *client, int selected_item, uint32_
/* XXX In theory, we could just rebuild the items and then use set_menu_items to replace the menu's items
* (still calling set_current_item), which would be more efficient than destroying/recreating the menu too. */
if (create_message_items(client) || create_messages_menu(client)) {
client_error("Failed to create message items or menu");
return -1;
}

Expand Down
1 change: 1 addition & 0 deletions imap.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ static void free_mailboxes(struct mailbox *mailboxes, int num_mailboxes)
for (i = 0; i < num_mailboxes; i++) {
free_mailbox_keywords(&mailboxes[i]);
free_if(mailboxes[i].name);
free_if(mailboxes[i].display);
}
free(mailboxes);
}
Expand Down
16 changes: 15 additions & 1 deletion menu.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,25 @@ void free_folder_items(struct client *client)
if (client->folders.items[i]) {
free_item(client->folders.items[i]);
}
free_if(client->mailboxes[i].display);
}
free(client->folders.items);
client->folders.items = NULL;
}
/* Iterate separately since client->num_mailboxes
* may not be the same as client->folders.n at this point,
* e.g. if a mailbox was completely deleted, and reconstruct_mailboxes
* was called to recreate the mailboxes array with a different size,
* the stale folder list contains more entries than there are mailboxes now.
*
* We also free the display name if needed in free_mailboxes,
* but both are needed. If the mailbox array shrunk during the resize,
* we'll take care of freeing that last index which now no longer exists.
* However, free_folder_items may be called many times during execution,
* not just when the mailboxes array is resized, and we still need
* to free all the existing ones here before calling create_folder_items again. */
for (i = 0; i < client->num_mailboxes; i++) {
free_if(client->mailboxes[i].display);
}
client->folders.n = 0;
}

Expand Down

0 comments on commit 23759f8

Please sign in to comment.