Skip to content

Commit efe131e

Browse files
authored
Merge pull request #109 from kareem-wolfssl/gh108
Wrap _delete/_copy class attrs in staticmethod so self isn't bound as an extra arg.
2 parents 292557e + 8bd719c commit efe131e

4 files changed

Lines changed: 200 additions & 15 deletions

File tree

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
# test_delete_descriptor_binding.py
2+
#
3+
# Copyright (C) 2026 wolfSSL Inc.
4+
#
5+
# This file is part of wolfSSL. (formerly known as CyaSSL)
6+
#
7+
# wolfSSL is free software; you can redistribute it and/or modify
8+
# it under the terms of the GNU General Public License as published by
9+
# the Free Software Foundation; either version 2 of the License, or
10+
# (at your option) any later version.
11+
#
12+
# wolfSSL is distributed in the hope that it will be useful,
13+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
# GNU General Public License for more details.
16+
#
17+
# You should have received a copy of the GNU General Public License
18+
# along with this program; if not, write to the Free Software
19+
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
20+
21+
"""
22+
Regression tests guarding against the Python descriptor-binding bug on
23+
``_delete`` / ``_copy`` class attributes.
24+
25+
Historically these were written as bare references to ``_lib`` functions::
26+
27+
class Random:
28+
_delete = _lib.wc_FreeRng
29+
30+
def __del__(self):
31+
self._delete(self.native_object)
32+
33+
If the underlying callable is ever a plain Python function (e.g. a mock,
34+
wrapper, or future CFFI change), the descriptor protocol turns
35+
``self._delete`` into a *bound method*, and ``self._delete(native)`` then
36+
calls ``fn(self, native)`` - passing ``self`` as an extra C argument.
37+
38+
The fix wraps the callable in ``staticmethod(...)`` at the class level so
39+
that attribute lookup never binds ``self``. These tests assert the fix
40+
stays in place and document the Python semantics it relies on.
41+
"""
42+
43+
# pylint: disable=redefined-outer-name
44+
45+
import inspect
46+
47+
import pytest
48+
49+
from wolfcrypt._ffi import lib as _lib
50+
51+
52+
def _static_attrs():
53+
"""Yield (cls, attr_name) pairs that must be staticmethod-wrapped."""
54+
from wolfcrypt.random import Random
55+
yield Random, "_delete"
56+
57+
if _lib.SHA_ENABLED:
58+
from wolfcrypt.hashes import Sha
59+
yield Sha, "_delete"
60+
yield Sha, "_copy"
61+
if _lib.SHA256_ENABLED:
62+
from wolfcrypt.hashes import Sha256
63+
yield Sha256, "_delete"
64+
yield Sha256, "_copy"
65+
if _lib.SHA384_ENABLED:
66+
from wolfcrypt.hashes import Sha384
67+
yield Sha384, "_delete"
68+
yield Sha384, "_copy"
69+
if _lib.SHA512_ENABLED:
70+
from wolfcrypt.hashes import Sha512
71+
yield Sha512, "_delete"
72+
yield Sha512, "_copy"
73+
if _lib.HMAC_ENABLED:
74+
from wolfcrypt.hashes import _Hmac
75+
yield _Hmac, "_delete"
76+
77+
if _lib.AESGCM_STREAM_ENABLED:
78+
from wolfcrypt.ciphers import AesGcmStream
79+
yield AesGcmStream, "_delete"
80+
if _lib.RSA_ENABLED:
81+
from wolfcrypt.ciphers import _Rsa
82+
yield _Rsa, "_delete"
83+
if _lib.ECC_ENABLED:
84+
from wolfcrypt.ciphers import _Ecc
85+
yield _Ecc, "_delete"
86+
if _lib.ED25519_ENABLED:
87+
from wolfcrypt.ciphers import _Ed25519
88+
yield _Ed25519, "_delete"
89+
if _lib.ED448_ENABLED:
90+
from wolfcrypt.ciphers import _Ed448
91+
yield _Ed448, "_delete"
92+
93+
94+
@pytest.mark.parametrize(
95+
"cls,attr",
96+
list(_static_attrs()),
97+
ids=lambda v: v if isinstance(v, str) else v.__name__,
98+
)
99+
def test_lib_fn_class_attr_is_staticmethod(cls, attr):
100+
"""The class attribute must be a ``staticmethod`` so that attribute
101+
access via an instance never triggers descriptor binding.
102+
103+
``inspect.getattr_static`` walks the MRO without invoking descriptors,
104+
so it returns the raw object (the ``staticmethod`` wrapper itself).
105+
"""
106+
raw = inspect.getattr_static(cls, attr)
107+
assert isinstance(raw, staticmethod), (
108+
"%s.%s must be wrapped in staticmethod(...) to prevent Python's "
109+
"descriptor protocol from injecting `self` as an extra positional "
110+
"argument when the underlying callable is a plain Python function "
111+
"(e.g. a test mock). Got %r." % (cls.__name__, attr, type(raw))
112+
)
113+
114+
115+
def test_descriptor_binding_semantics_documentation():
116+
"""Document the exact Python behavior the fix relies on.
117+
118+
Without ``staticmethod``, a Python-function class attribute becomes a
119+
bound method and leaks ``self`` into the call. ``staticmethod`` makes
120+
the descriptor return the underlying callable unchanged.
121+
"""
122+
received = []
123+
124+
def recorder(*args, **kwargs):
125+
received.append((args, kwargs))
126+
127+
class Buggy:
128+
_delete = recorder
129+
130+
def run(self):
131+
self._delete("native")
132+
133+
class Fixed:
134+
_delete = staticmethod(recorder)
135+
136+
def run(self):
137+
self._delete("native")
138+
139+
Buggy().run()
140+
buggy_args, _ = received[-1]
141+
assert len(buggy_args) == 2 and buggy_args[1] == "native", (
142+
"Sanity check failed: plain class-attribute Python function "
143+
"should have been bound and passed self as the first arg."
144+
)
145+
146+
Fixed().run()
147+
fixed_args, _ = received[-1]
148+
assert fixed_args == ("native",), (
149+
"staticmethod-wrapping should prevent self from being bound, "
150+
"so the callable receives only the intended positional argument."
151+
)
152+
153+
154+
def test_random_delete_receives_only_native_object():
155+
"""End-to-end behavioral check on the real ``Random`` class.
156+
157+
We substitute a plain Python recorder in place of the CFFI free
158+
function (wrapped in staticmethod, mirroring how the class itself
159+
stores it) and trigger the code path that calls ``self._delete``.
160+
The recorder must see exactly one positional argument - the
161+
``native_object`` - and never ``self``.
162+
"""
163+
from wolfcrypt.random import Random
164+
165+
received = []
166+
167+
def recorder(*args, **kwargs):
168+
received.append((args, kwargs))
169+
170+
original = inspect.getattr_static(Random, "_delete")
171+
try:
172+
Random._delete = staticmethod(recorder)
173+
r = Random()
174+
native = r.native_object
175+
r.__del__()
176+
r.native_object = None # prevent real cleanup on the way out
177+
assert received, "recorder was never called"
178+
args, kwargs = received[-1]
179+
assert kwargs == {}
180+
assert args == (native,), (
181+
"Random.__del__ must call _delete with only native_object, "
182+
"but got args=%r" % (args,)
183+
)
184+
finally:
185+
Random._delete = original

wolfcrypt/ciphers.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ class AesGcmStream:
397397
_key_sizes = [16, 24, 32]
398398
_native_type = "Aes *"
399399
# making sure _lib.wc_AesFree outlives Aes instances
400-
_delete = _lib.wc_AesFree
400+
_delete = staticmethod(_lib.wc_AesFree)
401401

402402
def __init__(self, key, IV, tag_bytes=16):
403403
"""
@@ -701,7 +701,7 @@ def __init__(self, rng=None):
701701
raise WolfCryptError("Key initialization error (%d)" % ret)
702702

703703
# making sure _lib.wc_FreeRsaKey outlives RsaKey instances
704-
_delete = _lib.wc_FreeRsaKey
704+
_delete = staticmethod(_lib.wc_FreeRsaKey)
705705

706706
def __del__(self):
707707
if self.native_object:
@@ -1060,7 +1060,7 @@ def __init__(self):
10601060
raise WolfCryptError("Invalid key error (%d)" % ret)
10611061

10621062
# making sure _lib.wc_ecc_free outlives ecc_key instances
1063-
_delete = _lib.wc_ecc_free
1063+
_delete = staticmethod(_lib.wc_ecc_free)
10641064

10651065
def __del__(self):
10661066
if self.native_object:
@@ -1426,7 +1426,7 @@ def __init__(self):
14261426
raise WolfCryptError("Invalid key error (%d)" % ret)
14271427

14281428
# making sure _lib.wc_ed25519_free outlives ed25519_key instances
1429-
_delete = _lib.wc_ed25519_free
1429+
_delete = staticmethod(_lib.wc_ed25519_free)
14301430

14311431
def __del__(self):
14321432
if self.native_object:
@@ -1626,7 +1626,7 @@ def __init__(self):
16261626
raise WolfCryptError("Invalid key error (%d)" % ret)
16271627

16281628
# making sure _lib.wc_ed448_free outlives ed448_key instances
1629-
_delete = _lib.wc_ed448_free
1629+
_delete = staticmethod(_lib.wc_ed448_free)
16301630

16311631
def __del__(self):
16321632
if self.native_object:

wolfcrypt/hashes.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,8 @@ class Sha(_Hash):
157157
digest_size = 20
158158
_native_type = "wc_Sha *"
159159
_native_size = _ffi.sizeof("wc_Sha")
160-
_delete = _lib.wc_ShaFree
161-
_copy = _lib.wc_ShaCopy
160+
_delete = staticmethod(_lib.wc_ShaFree)
161+
_copy = staticmethod(_lib.wc_ShaCopy)
162162

163163
def __del__(self):
164164
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
@@ -185,8 +185,8 @@ class Sha256(_Hash):
185185
digest_size = 32
186186
_native_type = "wc_Sha256 *"
187187
_native_size = _ffi.sizeof("wc_Sha256")
188-
_delete = _lib.wc_Sha256Free
189-
_copy = _lib.wc_Sha256Copy
188+
_delete = staticmethod(_lib.wc_Sha256Free)
189+
_copy = staticmethod(_lib.wc_Sha256Copy)
190190

191191
def __del__(self):
192192
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
@@ -213,8 +213,8 @@ class Sha384(_Hash):
213213
digest_size = 48
214214
_native_type = "wc_Sha384 *"
215215
_native_size = _ffi.sizeof("wc_Sha384")
216-
_delete = _lib.wc_Sha384Free
217-
_copy = _lib.wc_Sha384Copy
216+
_delete = staticmethod(_lib.wc_Sha384Free)
217+
_copy = staticmethod(_lib.wc_Sha384Copy)
218218

219219
def __del__(self):
220220
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
@@ -241,8 +241,8 @@ class Sha512(_Hash):
241241
digest_size = 64
242242
_native_type = "wc_Sha512 *"
243243
_native_size = _ffi.sizeof("wc_Sha512")
244-
_delete = _lib.wc_Sha512Free
245-
_copy = _lib.wc_Sha512Copy
244+
_delete = staticmethod(_lib.wc_Sha512Free)
245+
_copy = staticmethod(_lib.wc_Sha512Copy)
246246

247247
def __del__(self):
248248
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
@@ -403,7 +403,7 @@ class _Hmac(_Hash):
403403
digest_size = None
404404
_native_type = "Hmac *"
405405
_native_size = _ffi.sizeof("Hmac")
406-
_delete = _lib.wc_HmacFree
406+
_delete = staticmethod(_lib.wc_HmacFree)
407407

408408
def __del__(self):
409409
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):

wolfcrypt/random.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def __init__(self, nonce: __builtins__.bytes = b"", device_id: int = _lib.INVALI
4242
raise WolfCryptError("RNG init error (%d)" % ret)
4343

4444
# making sure _lib.wc_FreeRng outlives WC_RNG instances
45-
_delete = _lib.wc_FreeRng
45+
_delete = staticmethod(_lib.wc_FreeRng)
4646

4747
def __del__(self) -> None:
4848
if self.native_object:

0 commit comments

Comments
 (0)