Skip to content

Fix use after context release#368

Merged
nmoinvaz merged 3 commits into
zlib-ng:devfrom
tmtmazum:fix-use-after-context-release
Mar 6, 2019
Merged

Fix use after context release#368
nmoinvaz merged 3 commits into
zlib-ng:devfrom
tmtmazum:fix-use-after-context-release

Conversation

@tmtmazum

@tmtmazum tmtmazum commented Mar 4, 2019

Copy link
Copy Markdown

Fixes a couple of bugs on windows caught by AppVerifier

  • in function mz_crypt_pbkdf2(..), CryptDeleteHash was being called after the associated Crypt context was released
    • hmac1 and hmac2 are created using mz_crypt_hmac_init, so they each get their own context (provider) from the CryptAcquireContext(..) function
    • hmac3 is created using mz_crypt_hmac_copy, which creates a duplicate of hmac2's hash, sharing the same context
    • During destruction, hmac1 and hmac2 were being deleted first, followed by hmac3. This meant that the crypt contexts were released first by CryptReleaseContext. The subsequent call to CryptDestroyHash(..) on hmac3's hash would lead to UB as the associated context has already been released by that point
    • Fix: call mz_crypt_hmac_delete(..) on hmac3 first, so that the contexts are cleared at the end after the hashes are destroyed.
  • in function mz_crypt_hmac_copy, if target->hash already contained a hash prior to the function being called, that hash would be overwritten, without CryptDestroyHash being called on it.
    • Fix: check if target->hash is non-empty in mz_crypt_hmac_copy and call CryptDestroyHash on it

@nmoinvaz nmoinvaz changed the base branch from master to dev March 6, 2019 02:03
@nmoinvaz nmoinvaz merged commit 066157e into zlib-ng:dev Mar 6, 2019
@nmoinvaz

nmoinvaz commented Mar 6, 2019

Copy link
Copy Markdown
Member

Looks good. Thanks for the pull request!

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

Successfully merging this pull request may close these issues.

2 participants