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

Segfault when opening forwarded multipart email (as attachment) before sending #4177

Open
tomka opened this issue Jan 31, 2024 · 5 comments
Open
Labels

Comments

@tomka
Copy link

tomka commented Jan 31, 2024

Expected Behaviour

My goal was to forward only parts of a multipart email and select individual attachments. To do that I use the v key to go into attachment selection mode and select the individual parts I want to forward. In the simplest case I just keep the cursor on the root element of a multipart email. Then I press f to forward, add a recipient, edit the message and maybe open the forwarded attachments again before sending. Finally the mail should be sent.

Actual Behaviour

In the above description everything works apart from opening the forwarded message parts/attachments before sending by pressing Enter. Trying to do so results in a segmentation fault. This doesn't seem to happen when one tries to open "leaf nodes" in a multipart message (e.g. the actual HTML and text versions), as seen in the attachment view/list.

A coredump file is created and I can send it if it would be helpful (it's from a test email account). Initially I hoped to quickly peek into the source code, but couldn't get the function names and line numbers into neomutt to show up within a backtrace in cgdb. I compiled the latest main branch version with debug symbols (make EXTRA_CFLAGS="-g -O0") and started neomutt with debug logging (-d 5), but couldn't get anything meaningful from the coredump (nor the debug log). The core dump still only contains question marks instead of function names.

Steps to Reproduce

  1. Open the attachment list of a multipart mail (v key) and leave the cursor at the top/root element, leaving multipart attachments in branches below it (like with HTML+Text emails).
  2. Press f to forward the first attachment, use default "as attachment" and set a recipient.
  3. Exit message editor
  4. Select forwarded message attachment and press enter
  5. Segfault happens

How often does this happen?

Always with multipart emails. I simply created a HTML email in a web mail interface and send it to my address. Then I did the steps above on this email.

I can send a full example email id the trimmed down one I attached this issue isn't useful in itself: test.txt

When did it start to happen?

Not sure exactly when this started to happen. I experience occasional crashes with neomutt since quite a a while, unfortunately. There might be multiple issues, but it was just today that I seemed to notice a pattern and investigated this. I tested this with the latest release and the GitHub main branch HEAD.

NeoMutt Version

NeoMutt 20231221-72-ad8d0f-dirty
Copyright (C) 2015-2024 Richard Russon and friends
NeoMutt comes with ABSOLUTELY NO WARRANTY; for details type 'neomutt -vv'.
NeoMutt is free software, and you are welcome to redistribute it
under certain conditions; type 'neomutt -vv' for details.

System: Linux 6.6.10-arch1-1 (x86_64)
ncurses: ncurses 6.4.20230520 (compiled with 6.4.20230520)
libidn2: 2.3.4 (compiled with 2.3.4)
GPGME: 1.23.2
GnuTLS: 3.8.3
libnotmuch: 5.6.0
storage: kyotocabinet, gdbm, bdb, lmdb
compression: lz4, zlib, zstd

Configure options: --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib --gpgme --sqlite --autocrypt --lua --notmuch --gss --gnutls --sasl --with-idn2=/usr --disable-idn --idn2 --bdb --lmdb --kyotocabinet --gdbm --lz4 --zlib --zstd

Compilation CFLAGS: -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security         -fstack-clash-protection -fcf-protection -fno-delete-null-pointer-checks -D_ALL_SOURCE=1 -D_GNU_SOURCE=1 -D__EXTENSIONS__ -D_XOPEN_SOURCE_EXTENDED -I/usr/include -I/usr/include -I/usr/include -DNCURSES_WIDECHAR -I/usr/include -I/usr/include/p11-kit-1 -I/include -I/usr/include/db5.3 -I/usr/include -I/usr/include -O2

Compile options:
  +autocrypt +fcntl -flock -fmemopen +futimens +getaddrinfo +gnutls +gpgme
  -gsasl +gss +hcache -homespool +idn +inotify -locales_hack +lua -mixmaster
  +nls +notmuch -openssl +pgp +regex +sasl +smime +sqlite +truecolor

MAILPATH="/var/mail"
PKGDATADIR="/usr/share/neomutt"
SENDMAIL="/usr/sbin/sendmail"
SYSCONFDIR="/etc"

To learn more about NeoMutt, visit: https://neomutt.org
If you find a bug in NeoMutt, please raise an issue at:
    https://github.com/neomutt/neomutt/issues
or send an email to: <neomutt-devel@neomutt.org>

Even though the compilation options show no debug symbols, I added them according the neomutt debugging docs.

Extra Info

  • Arch Linux, Kernel 6.6.10
  • Normally, I run multiple neomutt copies at the same time, but this bug also happens with a single instance.
  • No tmux/screen was used.
  • I tested this both with IMAP and Maildir. The bugs happens in both cases.
@tomka tomka added the type:bug Bug label Jan 31, 2024
@dcpurton
Copy link
Collaborator

dcpurton commented Feb 1, 2024

I can reproduce this.

I suspect this use case was probably missed in the multipart code I originally worked on to fix up.

I notice that the attachment tree is wrongly displayed which further suggests that it was never implemented or tested and certainly indicates that the attachment structure is broken leading to an inevitable segfault.

In theory it's probably not too hard to fix, but I haven't looked at the code for a long time.

David

@roccoblues
Copy link
Member

I started looking into this. Here's a backtrace:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x70)
  * frame #0: 0x00000001000b0834 neomutt`mutt_attach_display_loop(sub=0x0000000140707c40, menu=0x00000001418062c0, op=31, e=0x0000000122a04d70, actx=0x00000001418c42e0, recv=false) at recvattach.c:981:23
    frame #1: 0x00000001000a0eb4 neomutt`op_display_headers(shared=0x000000014183e9b0, op=31) at functions.c:2023:5
    frame #2: 0x000000010009d7c0 neomutt`compose_function_dispatcher(win=0x0000000141893240, op=31) at functions.c:2156:12
    frame #3: 0x000000010009d108 neomutt`dlg_compose(e=0x0000000122a04d70, fcc=0x000000016fdfbbb8, flags='\0', sub=0x0000000140707c40) at dlg_compose.c:356:10
    frame #4: 0x000000010008d3c4 neomutt`mutt_send_message(flags=0, e_templ=0x0000000122a04d70, tempfile="/var/folders/6p/pdh_lfv143sc6nhsg0q54f6w0000gn/T/neomutt-dennis-schoen-501-35723-15226520958842890192", m=0x0000000000000000, ea=0x000000016fdfbc80, sub=0x0000000140707c40) at send.c:2713:9
    frame #5: 0x000000010003ea1c neomutt`attach_forward_bodies(fp=0x00000001dbe2e4e0, e=0x00000001223a6950, actx=0x0000000122a04ae0, b=0x00000001223a6f10, nattach=0) at recvcmd.c:627:3
    frame #6: 0x000000010003db04 neomutt`mutt_attach_forward(fp=0x00000001dbe2e4e0, e=0x00000001223a6950, actx=0x0000000122a04ae0, b=0x00000001223a6f10, flags=0) at recvcmd.c:799:5
    frame #7: 0x00000001000a90f8 neomutt`op_forward_message(priv=0x0000000122a04ca0, op=107) at functions.c:569:3
    frame #8: 0x00000001000a7fe4 neomutt`attach_function_dispatcher(win=0x0000000122a046d0, op=107) at functions.c:732:12
    frame #9: 0x00000001000a79a4 neomutt`dlg_attachment(sub=0x0000000140707c40, mv=0x00000001406cb990, e=0x00000001223a6950, fp=0x00000001dbe2e4e0, attach_msg=false) at dlg_attach.c:532:10
    frame #10: 0x0000000100050f94 neomutt`op_view_attachments(shared=0x0000000141826820, priv=0x0000000141826970, op=233) at functions.c:2496:5
    frame #11: 0x0000000100049ea8 neomutt`index_function_dispatcher(win=0x00000001418269a0, op=233) at functions.c:3285:12
    frame #12: 0x000000010004986c neomutt`dlg_index(dlg=0x0000000141826760, m_init=0x000000014060ab80) at dlg_index.c:1336:10
    frame #13: 0x0000000100027590 neomutt`main(argc=1, argv=0x000000016fdfd750, envp=0x000000016fdfd760) at main.c:1387:11
    frame #14: 0x00000001851d10e0 dyld`start + 2360

@dcpurton
Copy link
Collaborator

dcpurton commented Feb 9, 2024

The fix probably needs to happen in attach_forward_bodies (or somewhere in recvcmd.c).

Previously multipart mime types where just treated as a back box, so this probably worked. (Although there were a lot of bugs, so often things just worked by luck.) But now the compose code expects them to be attached as a tree structure.

The code probably more or less exists because the send from a postponed message code is capable of creating the attachment tree from a multipart attachment. But there are more cases that potentially need to be handled here. You can forward anything, but only create multipart/alternative, multipart/multilingual and multipart/related. So there are possibly unwise assumptions about what kinds of multipart attachments will appear while composing. The postpone code strips multipart/mixed and crypt multiparts.

It's conceivable you might want to forward a multipart/mixed attachment I guess (not sure?). Forwarding an encrypted attachment seems somewhat pointless, but maybe there is a use case. I can see how you might want to forward a signed attachment. Although the receiver would need to find some custom way of verifying the signature.

All this is to say that it's not quite as simple to fix as I hoped. Some design and behaviour decisions need to be made. @roccoblues if you want to do it you are welcome :). I can tinker, but no timeline promises.

@roccoblues
Copy link
Member

@dcpurton Thanks for the info. I have the bad feeling this might be a bit over my head but I'll at least try to understand the problem.

Previously multipart mime types where just treated as a back box, so this probably worked.

I stepped through the code and mutt_copy_body is used to copy the body. It seems like this is doing what you describe above - black box copy. So we have to update or replace that with a version that actually copies the tree structure (loop through b->next)?

The code probably more or less exists because the send from a postponed message code is capable of creating the attachment tree from a multipart attachment

Can you point me to that code? Thanks!

@dcpurton
Copy link
Collaborator

@roccoblues , the code I wrote to handle recalling postponed and resending messages is in 9350145

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants