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.

Sign up for GitHub

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

Jump to bottom

psa_generate_key for a symmetric key of ≥1025 bytes causes a double free #207

Closed
gilles-peskine-arm opened this issue Aug 7, 2019 · 2 comments
Closed

psa_generate_key for a symmetric key of ≥1025 bytes causes a double free #207

gilles-peskine-arm opened this issue Aug 7, 2019 · 2 comments
Labels
api-spec Issue or PR about the PSA specifications bug Something isn't working Jira status: CLOSED mirrored type: bug

Comments

@gilles-peskine-arm
Copy link
Collaborator

Description

Steps to reproduce: on the psa-api-1.0-beta branch, call psa_generate_key() with any symmetric key type that allows large key sizes (e.g. PSA_KEY_TYPE_RAW, PSA_KEY_TYPE_HMAC, PSA_KEY_TYPE_DERIVE) and a size of 8200 bits or more (>1024 bytes). This causes a double free inside psa_generate_key.

Impacted product: This only affects the psa-api-1.0-branch, the offending code has never been merged to the Mbed Crypto development branch and thus this does not affect any Mbed Crypto release. The bug was introduced in #263, merged on 2019-05-03, earliest tag psa-api-1.0-beta3.

Impact: this is a double free(), so it can lead to arbitrary code execution. However, the exploitability is low, possibly nonexistent in single-threaded applications with many free() implementations, because there is only a very short window of time between the two free() calls, with no other call to malloc or free in between in the same thread.

Affected applications: The bug is triggered by a failure of psa_generate_random during key generation for a symmetric key (e.g. HMAC or derive, but not asymmetric keys such as RSA). This can happen in two ways:

  • If the key size is 1025 bytes or more, due to psa_generate_random fails on large requests #206.
  • If the CTR_DRBG generator needs reseeding and the entropy source(s) fail, causing reseeding to fail. By default, the reseed interval for CTR_DRBG in Mbed Crypto is 10000 bytes. (Note that this is the total number of bytes generated internally, which is typically larger than the total number of bytes requested because CTR_DRBG generates a whole new block even when less than one block is requested.)

Issue request type

[ ] Question
[ ] Enhancement
[x] Bug
@gilles-peskine-arm gilles-peskine-arm added bug Something isn't working api-spec Issue or PR about the PSA specifications labels Aug 7, 2019
gilles-peskine-arm added a commit to gilles-peskine-arm/mbed-crypto that referenced this issue Aug 7, 2019
@gilles-peskine-arm
Fix double free in psa_generate_key when psa_generate_random fails
a6b2f60 
When psa_generate_random fails, psa_generate_key_internal frees the
key buffer but a the pointer to the now-freed buffer in the slot. Then
psa_generate_key calls psa_fail_key_creation which sees the pointer
and calls free() again.

This bug was introduced by ff5f0e7
"Implement atomic-creation psa_{generate,generator_import}_key" which
changed how psa_generate_key() cleans up on errors. I went through the
code and could not find a similar bug in cleanup on an error during
key creation.

Fix ARMmbed#207
This was referenced Aug 7, 2019
Merged
Open
@ciarmcom
Copy link
Member

ciarmcom commented Aug 7, 2019

Internal Jira reference: https://jira.arm.com/browse/IOTCRYPT-855

@gilles-peskine-arm
Copy link
Collaborator Author

Fixed — #197 fixes this and is now merged to psa-api-1.0-beta, which is the sole medium-term branch that had this bug,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api-spec Issue or PR about the PSA specifications bug Something isn't working Jira status: CLOSED mirrored type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ciarmcom @gilles-peskine-arm