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

Coverity scan error - Out-of-bounds access (ARRAY_VS_SINGLETON)- in Ratchet.cpp file #94

Open
sarigavs-jeppesen opened this issue Sep 8, 2024 · 0 comments

Comments

@sarigavs-jeppesen
Copy link

Getting error CID 239912: (#1 of 1): Out-of-bounds access (ARRAY_VS_SINGLETON) , callee_ptr_arith: Passing skipped to function erase which uses it as an array. This might corrupt or misinterpret adjacent memory locations in Ratchet.cpp file while running Coverity scan tool.

Find the below code with comments(in bold) from Coverity tool,

std::size_t olm::Ratchet::decrypt(
505 std::uint8_t const * input, std::size_t input_length,
506 std::uint8_t * plaintext, std::size_t max_plaintext_length
507) {
508 olm::MessageReader reader;
509 olm::decode_message(
510 reader, input, input_length,
511 ratchet_cipher->ops->mac_length(ratchet_cipher)
512 );
513
1. Condition reader.version != 3 / ::PROTOCOL_VERSION /, taking false branch.
514 if (reader.version != PROTOCOL_VERSION) {
515 last_error = OlmErrorCode::OLM_BAD_MESSAGE_VERSION;
516 return std::size_t(-1);
517 }
518
2. Condition !reader.has_counter, taking false branch.
3. Condition !reader.ratchet_key, taking false branch.
4. Condition !reader.ciphertext, taking false branch.

519 if (!reader.has_counter || !reader.ratchet_key || !reader.ciphertext) {
520 last_error = OlmErrorCode::OLM_BAD_MESSAGE_FORMAT;
521 return std::size_t(-1);
522 }
523
524 std::size_t max_length = ratchet_cipher->ops->decrypt_max_plaintext_length(
525 ratchet_cipher,
526 reader.ciphertext_length
527 );
528
5. Condition max_plaintext_length < max_length, taking false branch.
529 if (max_plaintext_length < max_length) {
530 last_error = OlmErrorCode::OLM_OUTPUT_BUFFER_TOO_SMALL;
531 return std::size_t(-1);
532 }
533
6. Condition reader.ratchet_key_length != 32, taking false branch.
534 if (reader.ratchet_key_length != CURVE25519_KEY_LENGTH) {
535 last_error = OlmErrorCode::OLM_BAD_MESSAGE_FORMAT;
536 return std::size_t(-1);
537 }
538
539 ReceiverChain * chain = nullptr;
540
7. Iterating over another element of this->receiver_chains.
541 for (olm::ReceiverChain & receiver_chain : receiver_chains) {
8. Condition 0 == memcmp(receiver_chain.ratchet_key.public_key, reader.ratchet_key, 32), taking true branch.
542 if (0 == std::memcmp(
543 receiver_chain.ratchet_key.public_key, reader.ratchet_key,
544 CURVE25519_KEY_LENGTH
545 )) {
546 chain = &receiver_chain;
9. Breaking from loop.
547 break;
548 }
549 }
550
551 std::size_t result = std::size_t(-1);
552
10. Condition !chain, taking false branch.
553 if (!chain) {
554 result = verify_mac_and_decrypt_for_new_chain(
555 this, reader, plaintext, max_plaintext_length
556 );
11. Condition chain->chain_key.index > reader.counter, taking true branch.
557 } else if (chain->chain_key.index > reader.counter) {
558 /
Chain already advanced beyond the key for this message
559 * Check if the message keys are in the skipped key list. */
*12. Iterating over another element of this->skipped_message_keys.
13. address_of: Taking address with __begin3 yields a singleton pointer.
14. assign: Assigning: skipped = __begin3.

560 for (olm::SkippedMessageKey & skipped : skipped_message_keys) {
15. Condition reader.counter == skipped.message_key.index, taking true branch.
16. Condition 0 == memcmp(skipped.ratchet_key.public_key, reader.ratchet_key, 32), taking true branch.

561 if (reader.counter == skipped.message_key.index
562 && 0 == std::memcmp(
563 skipped.ratchet_key.public_key, reader.ratchet_key,
564 CURVE25519_KEY_LENGTH
565 )
566 ) {
567 /
Found the key for this message. Check the MAC. /
568
569 result = verify_mac_and_decrypt(
570 ratchet_cipher, skipped.message_key, reader,
571 plaintext, max_plaintext_length
572 );
573
17. Condition result != 18446744073709551615UL / (size_t)-1 /, taking true branch.
574 if (result != std::size_t(-1)) {
575 /
Remove the key from the skipped keys now that we've
576 * decoded the message it corresponds to. */
577 olm::unset(skipped);

CID 239912: (#1 of 1): Out-of-bounds access (ARRAY_VS_SINGLETON)
18. callee_ptr_arith: Passing skipped to function erase which uses it as an array. This might corrupt or misinterpret adjacent memory locations.[show details]

578 skipped_message_keys.erase(&skipped);
579 return result;
580 }
581 }
582 }
583 } else {
584 result = verify_mac_and_decrypt_for_existing_chain(
585 this, chain->chain_key,
586 reader, plaintext, max_plaintext_length
587 );
588 }
589
590 if (result == std::size_t(-1)) {
591 last_error = OlmErrorCode::OLM_BAD_MESSAGE_MAC;
592 return std::size_t(-1);
593 }
594
595 if (!chain) {
596 /
They have started using a new ephemeral ratchet key.
597 * We need to derive a new set of chain keys.
598 * We can discard our previous ephemeral ratchet key.
599 * We will generate a new key when we send the next message. */
600
601 chain = receiver_chains.insert();
602 olm::load_array(chain->ratchet_key.public_key, reader.ratchet_key);
603
604 // TODO: we've already done this once, in
605 // verify_mac_and_decrypt_for_new_chain(). we could reuse the result.
606 create_chain_key(
607 root_key, sender_chain[0].ratchet_key, chain->ratchet_key,
608 kdf_info, root_key, chain->chain_key
609 );
610
611 olm::unset(sender_chain[0]);
612 sender_chain.erase(sender_chain.begin());
613 }
614
615 while (chain->chain_key.index < reader.counter) {
616 olm::SkippedMessageKey & key = *skipped_message_keys.insert();
617 create_message_keys(chain->chain_key, kdf_info, key.message_key);
618 key.ratchet_key = chain->ratchet_key;
619 advance_chain_key(chain->chain_key, chain->chain_key);
620 }
621
622 advance_chain_key(chain->chain_key, chain->chain_key);
623
624 return result;
625}

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

No branches or pull requests

1 participant