search

hohl / MIHCrypto

OpenSSL wrapper for Objective-C [cryptography]
MIT License
341 stars 68 forks source link

memory leak found in MIHRSAPrivateKey #60

Closed zty0 closed 4 years ago

zty0 commented 4 years ago
Screen Shot 2020-04-30 at 10 24 47 AM

Is there a reason that this line of code EVP_PKEY_free(pkey); is commented?

zty0 commented 4 years ago

not only pkey is leaked, privateBIO is also leaked in this code

hohl commented 4 years ago

Thank you for pointing that out. Looking at the commit history, I am to blame for the uncommented line myself, but as this happened 6 years ago, I have no clue anymore why I did that. Let my quickly check out, whether it would break anything when I uncomment that part, but I guess it is needed by EVP_PKEY_get1_RSA as that seems to only use it by reference.

hohl commented 4 years ago

Indeed, privateBIO also seems to be leaked a few lines below (in - (NSData *)dataValue) too.

hohl commented 4 years ago

As EVP_PKEY_free does references counting internally (according to the OpenSSL docs), it should be fine to call it in the @finally block, so I really can't see why that line was uncommented 6 years ago.

zty0 commented 4 years ago

I checked source code of Openssl a few hours ago, I think it should be safe to just free privateBIO and pkey in the @finally block.

zty0 commented 4 years ago

@hohl actually, you only fixed this problem in MIHRSAPublicKey.m MIHRSAPrivateKey.m has the same problem

zty0 commented 4 years ago

oh, my bad. I missed the other commit reference. closing the issue