AntonKueltz / fastecdsa

Python library for fast elliptic curve crypto
https://pypi.python.org/pypi/fastecdsa
The Unlicense
264 stars 77 forks source link

Memory leak #6

Closed targon closed 2 years ago

targon commented 7 years ago

Some of the functions seem to leak memory. The test program below will continuously eat up more and more memory.

I suspect the issue is in your use of Py_BuildValue without freeing the char* afterwards. For example, in curvemath_mul, resultX and resultY are allocated but never freed.

#!/usr/bin/env python

import fastecdsa.curvemath, fastecdsa.curve, gc

sx = str(fastecdsa.curve.secp256k1.G[0])
sy = str(fastecdsa.curve.secp256k1.G[1])
sz = str(2**255-1)

while True:
    fastecdsa.curvemath.mul(sx, sy, sz, 'secp256k1')
    gc.collect()
AntonKueltz commented 7 years ago

Thanks for the info, I suspect this was introduced via the branch that added koblitz curves. I'll take a look and submit a patch within the next day or two.

AntonKueltz commented 7 years ago

Issue seems to be coming from the memory management in the binary field logic, in the process of mitigating.

Edit: This one is going to take some time to fully mitigate. Until then all binary curves (K***) are not recommended for use.

targon commented 7 years ago

Attached patch fixes a number of memory leaks. After applying it, I can run curvemath.mul, curvemath.add and _ecdsa.verify in loops without increasing the memory usage.

memleak.patch.txt

Can you please check & merge?

AntonKueltz commented 7 years ago

Will verify patch tonight and merge, thanks!

AntonKueltz commented 7 years ago

Changes merged, thanks again for the patch.

Keeping issue open as there is still major memory leakage in the binary curves. To replicate on macOS take the following steps:

Keeping this issue open until this is resolved.

EggPool commented 5 years ago

I gave this a try with current version and secp256k1: signature checks do not leak anymore, but signing does.

import gc, os, psutil
from hashlib import sha1

from fastecdsa import ecdsa
from fastecdsa.curve import secp256k1
from fastecdsa.keys import gen_keypair

message = b"Sample message for signature testing"
priv, pub = gen_keypair(secp256k1)

CHECK_SIGN = True
CHECK_VERIFY = False

process = psutil.Process(os.getpid())
r, s = ecdsa.sign(message, priv, hashfunc=sha1, curve=secp256k1)

while True:
    print(process.memory_info().rss)
    if CHECK_SIGN:
        for i in range(1000):
            r, s = ecdsa.sign(message, priv, hashfunc=sha1, curve=secp256k1)
        gc.collect()
    if CHECK_VERIFY:
        for i in range(1000):
            check_sig = ecdsa.verify((r, s), message, pub, curve=secp256k1, hashfunc=sha1)
        gc.collect()
AntonKueltz commented 4 years ago

Verified memory leak. Will need to write some C test code and profile with tooling.