Closed Bug 1646367 Opened 3 months ago Closed 3 months ago

Crash after: delete OpenPGP key, then import again

Categories

(MailNews Core :: Security: OpenPGP, defect, P1)

Tracking

(thunderbird78 fixed, seamonkey2.53 affected)

RESOLVED FIXED
Thunderbird 79.0
Tracking Status
thunderbird78 --- fixed
seamonkey2.53 --- affected

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

(Keywords: crash)

Crash Data

Attachments

(3 files)

I have the key for kaie at thunderbird already imported at the time I start TB.
Open key management, delete that key.
Go to the email that has that key attached.
Right click import.
Crash

stack:

#5  0x0000780d678d6730 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#6  0x0000780d4eea9432 in std::char_traits<char>::assign(char&, char const&) (__c1=<optimized out>, __c2=<optimized out>) at /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/char_traits.h:287
#7  0x0000780d4eea9432 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_set_length(unsigned long) (this=0x780d50509198, __n=0)
    at /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/basic_string.h:206
#8  0x0000780d4eea9432 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)
    (this=0x780d50509198, __str=...) at /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/basic_string.h:752
#9  0x0000780d4eea649e in pgp_revoke_t::operator=(pgp_revoke_t&&) (this=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/types.h:364
#10 0x0000780d4eea649e in pgp_key_clear_revokes(pgp_key_t*) (key=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:234
#11 0x0000780d4eea6028 in pgp_subkey_refresh_data(pgp_key_t*, pgp_key_t*) (sub=0x780d50506010, key=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:1158
#12 0x0000780d4eea89ce in pgp_key_revalidate_updated(pgp_key_t*, rnp_key_store_t*) (key=0x780d446e6010, keyring=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:2090
#13 0x0000780d4eebf17d in rnp_key_store_import_key(rnp_key_store_t*, pgp_key_t*, bool, pgp_key_import_status_t*) (keyring=0x780d4fe79a00, srckey=<optimized out>, pubkey=<optimized out>, status=0x7ffdd109b580)
    at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librekey/rnp_key_store.cpp:557
#14 0x0000780d4eeac6c9 in rnp_import_keys(rnp_ffi_t, rnp_input_t, uint32_t, char**) (ffi=0x780d4f68af90, input=<optimized out>, flags=<optimized out>, results=0x0)
    at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/rnp.cpp:1311

found by Magnus while testing bug 1646331 (which is difficult to reproduce)

Can reproduce on 78 beta branch, too.

Nickolay, could you please have a look at this crash stack? Is it sufficient to allow you to identify the possible cause for the crash?

Priority: -- → P1
Version: unspecified → 78
Flags: needinfo?(o.nickolay)

Kai, does this happen with every key within this scenario, or only with some particular one?
In the last case could I have that particular key right as it was sent in email?

Flags: needinfo?(o.nickolay)

This is the key that triggers the crash with the stack above (delete, then import).

When testing a different key (this one), I ran into a crash with a different stack:

#5  0x00007b8dfa880730 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#6  0x00007b8de19a6271 in pgp_subkey_validate_self_signatures(pgp_key_t*, pgp_key_t*) (sub=0x7b8dd8243010, key=0x7b8ddadb5010) at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:1103
#7  0x00007b8de19a8557 in pgp_key_validate_subkey(pgp_key_t*, pgp_key_t*) (subkey=0x7b8dd8243010, key=0x7b8ddadb5010) at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:2033
#8  0x00007b8de19a89c3 in pgp_key_revalidate_updated(pgp_key_t*, rnp_key_store_t*) (key=0x7b8ddadb5010, keyring=<optimized out>) at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:2089
#9  0x00007b8de19bf17d in rnp_key_store_import_key(rnp_key_store_t*, pgp_key_t*, bool, pgp_key_import_status_t*) (keyring=0x7b8de284ae00, srckey=<optimized out>, pubkey=<optimized out>, status=0x7ffc27bbe760)
    at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/librekey/rnp_key_store.cpp:557
#10 0x00007b8de19ac6c9 in rnp_import_keys(rnp_ffi_t, rnp_input_t, uint32_t, char**) (ffi=0x7b8de2774200, input=<optimized out>, flags=<optimized out>, results=0x0)
    at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/rnp.cpp:1311

I hope you are able to reproduce easily.

If not, we'll have to investigate the TB code.
I cannot completely rule out the possibility that there's a bug in the C API definitions that we use in the JavaScript to C bridge.

Thanks, Kai and Magnus. I was able to reproduce this issue. Recently reported crashes bp-4355857e-94f1-4453-9e74-a72e00200615 and bp-79add039-c2c4-4b5f-99af-03a4c0200615 are caused by this issue.
Will let you know once fix is available.

Btw, just a side note - did you consider to build Thunderbird betas with sanitizers? That could give a lot more information, allowing to find the crash source much quicker.

(In reply to Nickolay Olshevsky from comment #5)

Thanks, Kai and Magnus. I was able to reproduce this issue. Recently reported crashes bp-4355857e-94f1-4453-9e74-a72e00200615 and bp-79add039-c2c4-4b5f-99af-03a4c0200615 are caused by this issue.

https://github.com/rnpgp/rnp/issues/1171

Note: if you add "bp-" (for breakpad, old name) in front of the crash id bugzilla will autolink it. As bp-4355857e-94f1-4453-9e74-a72e00200615 and bp-79add039-c2c4-4b5f-99af-03a4c0200615

Update: crash source was detected and fixed, however need some more time to update tests and add some new functionality.

Issue was fixed in PR #1176 (https://github.com/rnpgp/rnp/pull/1176).

Kai: please also note the new flag for rnp_key_remove(). Previously it was deleting primary key without the subkeys.

(In reply to Nickolay Olshevsky from comment #10)

Kai: please also note the new flag for rnp_key_remove(). Previously it was deleting primary key without the subkeys.

Thanks a lot for making me aware of this detail!

Depends on: 1647671
Assignee: nobody → kaie
Status: NEW → ASSIGNED

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/552a164af0b8
Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 79.0

Comment on attachment 9158478 [details]
Bug 1646367 - Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Required for correct delete/re-import functionality of OpenPGP keys.

Attachment #9158478 - Flags: approval-comm-beta?

Comment on attachment 9158478 [details]
Bug 1646367 - Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Approved for beta

Attachment #9158478 - Flags: approval-comm-beta? → approval-comm-beta+
Crash Signature: [@ pgp_subkey_validate_self_signatures ] [@ pgp_key_clear_revokes ]
Keywords: crash
You need to log in before you can comment on or make changes to this bug.