Repository navigation
Conversation
A PreKeySignalMessage's one-time prekey id is optional on the wire, and libsignal-protocol-c's pre_key_signal_message_get_pre_key_id() asserts it is present. omaq_ratchet_decrypt() called the getter without checking pre_key_signal_message_has_pre_key_id() first, so a message from an accepted contact without that id aborted the helper. An id of 0 is also refused: it is the "any id" sentinel of prekey_reservation_find() and consumed_prekey_find(), so it skipped the per-peer reservation check. OmaQ only issues ids in [1, 0xFFFFFF], so neither form is ever sent by an OmaQ client. pre_rm() refuses id 0 too. The message is rejected through the existing error path (-1), without recovery or session reset. ratchet_prekey_test forges both forms, on a first message and on a follow-up prekey message, and checks that no prekey is consumed and that the genuine messages still decrypt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed privately, this is the fix for the helper abort on malformed prekey messages.
Problem. In
omaq_ratchet_decrypt, aCIPHERTEXT_PREKEY_TYPEmessage goes topre_key_signal_message_get_pre_key_id()without apre_key_signal_message_has_pre_key_id()check. The id is optional in the protobuf, and the libsignal-protocol-c getter asserts it is present, so a message from an accepted contact without that id aborts the helper. An id of0also matches the "any id" sentinel ofprekey_reservation_find/consumed_prekey_findand skips the per-peer reservation check.Fix.
-1), without recovery or session reset.pre_rmrefuses id 0 too.Test.
ratchet_prekey_testgainsprekey_id_guard. It forges both forms, missing id and id 0, on a first message and on a follow-up prekey message. It checks that:Verified on Arch with Signal enabled:
ratchet_prekey_testaborts (SIGABRT) without the fix and passes with it (ASan/UBSan,-Werror).make testis otherwise unchanged. Onlychat-transcript-layout,panel-request-focusandchat-input-scrollfail, the same on unpatchedmainin a fresh checkout (generated emoji assets missing).helper-updatefailed once in the full run, then passed 3/3 on its own with the patch.🤖 Generated with Claude Code