1673236 - RNP-01-007 WP1 RNP: encrypt_secret_key() does not wipe keybuf from memory (Low)

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Wayne Mery (:wsmwk)]

While auditing the provided sources for potential vulnerabilities, it was discovered that the function encrypt_secret_key() used for encrypting data does not properly clear the derived key stored in keybuf before the function returns. keybuf is reserved on the stack, and in the case of encryption successful, it does not get wiped from memory using pgp_forget(). It has to be noted that the buffer gets properly zeroed in memory in the error case, as depicted in the code snippet below.

Affected File:
rnp/src/librepgp/stream-key.cpp

Affected Code:

Rnp_result_t
encrypt_secret_key(pgp_key_pkt_t *key, const char *password, rng_t *rng)
{
       uint8_t keybuf[PGP_MAX_KEY_SIZE];
        [...]
        /* derive key */
        if (!pgp_s2k_derive_key(&key->sec_protection.s2k, password, keybuf, keysize)) {
           RNP_LOG("failed to derive key");
           ret = RNP_ERROR_BAD_PARAMETERS;
           goto error;
       } 
       [...]
      if (!pgp_cipher_cfb_start(
             &crypt, key->sec_protection.symm_alg, keybuf, key->sec_protection.iv)) {
             RNP_LOG("failed to start cfb encryption");
             ret = RNP_ERROR_DECRYPT_FAILED;
             goto error;
        }
       pgp_cipher_cfb_encrypt(&crypt, body.data, body.data, body.len);
       pgp_cipher_cfb_finish(&crypt);
       key->sec_data = body.data;
       key->sec_len = body.len;
       [...]
       return RNP_SUCCESS;
error:
     pgp_forget(keybuf, sizeof(keybuf));
     pgp_forget(body.data, body.len);
     free_packet_body(&body);
     return ret;
}

Sensitive data, such as keys, passwords, etc., should be wiped from memory using the pgp_forget() method in order to reduce the potential risk of disclosing sensitive information to local attackers who are in position to read the process memory.

Comment 1

[Magnus Melin [:mkmelin]]

Is there an RNP issue filed for this?

Comment 2

[Nickolay Olshevsky]

Magnus, not in the public repository. I will post a link to PR here once issue is fixed.

Comment 3

[Nickolay Olshevsky]

Fixed via https://github.com/rnpgp/rnp/pull/1350

Comment 4

[Kai Engert (:KaiE:)]

Not yet fixed in Thunderbird.
Needs a RNP snapshot newer than 2020-11-30

Comment 5

[Kai Engert (:KaiE:)]

Wayne, the uplift request in bug 1692909 is to fix this security issue.

Comment 6

[Kai Engert (:KaiE:)]

fixed by bug 1692909