Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use after free of a->mailbox due to missing strdup #3090

Merged
merged 1 commit into from Oct 23, 2021

Conversation

diabonas
Copy link
Contributor

@diabonas diabonas commented Oct 23, 2021

Commit 87ae932 changed crypt_add_string_to_hints(a->mailbox, &hints) to mutt_list_insert_tail(&hints, a->mailbox). However, there is a behavioural difference between the two functions: crypt_add_string_to_hints() adds a copy of the string to the list, while mutt_list_insert_tail() does not. This leads to a crash because the original a->mailbox is freed prematurely as part of the hints list. Fix this by adding a copy of the original to the list instead.

Note that commit 87ae932 originally came from Mutt. Upstream is not affected by this however because their mutt_add_list() functions always copies the data.

Downstream tracking issue in Arch Linux: FS#72525

Commit 87ae932 ("Directly add full mailbox to
GPG search hints") changed crypt_add_string_to_hints(a->mailbox, &hints) to
mutt_list_insert_tail(&hints, a->mailbox). However, there is a behavioural
difference between the two functions: crypt_add_string_to_hints() adds a copy
of the string to the list, while mutt_list_insert_tail() does not. This leads
to a crash because the original a->mailbox is freed prematurely as part of the
hints list. Fix this by adding a copy of the original to the list instead.

Note that commit 87ae932 originally came from
Mutt. Upstream is not affected by this however because their mutt_add_list()
functions always copies the data.
@flatcap flatcap added the bug:crash Causes NeoMutt to crash label Oct 23, 2021
Copy link
Member

@flatcap flatcap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh!
Thanks for the fix, @diabonas.

@flatcap flatcap merged commit a4a02a4 into neomutt:master Oct 23, 2021
@diabonas diabonas deleted the fix-mailbox-strdup branch October 23, 2021 13:35
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Oct 23, 2021
Credits @diabonas
neomutt/neomutt#3090

git-svn-id: file:///srv/repos/svn-community/svn@1032960 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Oct 23, 2021
Credits @diabonas
neomutt/neomutt#3090


git-svn-id: file:///srv/repos/svn-community/svn@1032960 9fca08f4-af9d-4005-b8df-a31f2cc04f65
@flatcap flatcap linked an issue Oct 28, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:crash Causes NeoMutt to crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opportunistic PGP encryption with keyless recipient causes core dump
2 participants