search

pqc-thunderbird / rnp

Manual clone of the repository https://github.com/rnpgp/rnp
Other
0 stars 0 forks source link

more readable handling of session key length in stream-parse.cpp:encrypted_try_key() #82

Closed falko-strenzke closed 1 month ago

falko-strenzke commented 2 months ago

in stream-parse.cpp:encrypted_try_key() we find the code:

#if defined(ENABLE_CRYPTO_REFRESH) || defined(ENABLE_PQC)
    if (do_encrypt_pkesk_v3_alg_id(sesskey->alg))
#endif
    {
        sesskey->salg = static_cast<pgp_symm_alg_t>(decbuf[0]);
    }
    size_t keylen = pgp_key_size(sesskey->salg);

This looks like keylen will receive an invalid value (based on unassigned memory?) in case of v6 PKESK.

@TJ-91

TJ-91 commented 2 months ago

pgp_key_size will return 0 since sesskey->salg is initialized with PGP_SA_UNKNOWN and later on the keylen will be checked with the decrypted result later.

Two improvements for readability and maintainability possible:

1) Make the check earlier (decbuf and param->aead_hdr.ealg are available already) 2) Split PKESKv3/SEIPDv1 and PKESKv6/SEIPDv2 cases completely

The first one should be little effort

TJ-91 commented 1 month ago

Improved a bit in update-draft-05 branch