SFT-8173: validate scalar and point buffer lengths in the ECDSA bindings
What changed, and why it matters
This commit fixes a buffer length bug in the firmware's cryptographic code. Two functions that perform elliptic-curve math were reading exactly 32 bytes from caller-supplied buffers without first checking that the buffers were actually 32 bytes long. A shorter buffer could cause the code to read beyond its end, which on a hardware wallet could leak secret data or crash the device. The fix adds explicit length checks that reject any input that is not exactly 32 bytes, and adds tests to confirm the checks work.
Treat this as a security-hardening fix with memory-safety implications. Ensure the patch is included in the next firmware release, run the new unit tests, and audit other trezorcrypto bindings for similar fixed-size reads without length checks.
Security signals we found
Out-of-bounds read in cryptographic binding (C extension reading fixed 32 bytes without validating buffer length)
Potential information disclosure or fault/crash from malformed scalar or point buffers
Missing input validation in ECDSA low-level primitives exposed to Python
Unit tests added to enforce length validation and prevent regression
Evidence from the diff
In extmod/trezor-firmware/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ecdsa.h, mod_trezorcrypto_ecdsa_scalar_multiply() and mod_trezorcrypto_ecdsa_point_add() use mp_get_buffer_raise() to obtain a pointer/length pair, then call bn_read_be(), which always reads 32 bytes. Previously, buffers shorter than 32 bytes would cause an out-of-bounds read. The patch adds length equality checks (len != 32) for the scalar and for all four point coordinates, raising ValueError on mismatch. A new unit test file, ecdsa_bindings.py, exercises valid operations (1*G, G+2G=3G) and verifies that lengths 0, 1, 31, 33, and 64 are rejected for every parameter position, and that rejection does not corrupt subsequent valid calls.
Changed components
extmod/trezor-firmware/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ecdsa.hports/stm32/boards/Passport/modules/tests/test_unit.pyports/stm32/boards/Passport/modules/tests/unit/ecdsa_bindings.pytrezorcrypto.ecdsa Python module bindingsInspect captured patch +86 / −0
### extmod/trezor-firmware/core/embed/extmod/modtrezorcrypto/modtrezorcrypto-ecdsa.h
@@ -44,6 +44,12 @@ STATIC mp_obj_t mod_trezorcrypto_ecdsa_scalar_multiply(mp_obj_t k_obj) {
mp_buffer_info_t k_buf;
mp_get_buffer_raise(k_obj, &k_buf, MP_BUFFER_READ);
+ // bn_read_be() always reads 32 bytes, so reject anything else rather than
+ // reading past the end of the buffer.
+ if (k_buf.len != 32) {
+ mp_raise_ValueError(MP_ERROR_TEXT("Invalid length of scalar"));
+ }
+
// Convert k to a bignum
bignum256 k;
bn_read_be((uint8_t *)k_buf.buf, &k);
@@ -94,6 +100,21 @@ STATIC mp_obj_t mod_trezorcrypto_ecdsa_point_add(size_t n_args, const mp_obj_t *
mp_buffer_info_t y2_buf;
mp_get_buffer_raise(args[3], &y2_buf, MP_BUFFER_READ);
+ // bn_read_be() always reads 32 bytes from each coordinate, so reject any
+ // other length rather than reading past the end of the buffer.
+ if (x1_buf.len != 32) {
+ mp_raise_ValueError(MP_ERROR_TEXT("Invalid length of x1"));
+ }
+ if (y1_buf.len != 32) {
+ mp_raise_ValueError(MP_ERROR_TEXT("Invalid length of y1"));
+ }
+ if (x2_buf.len != 32) {
+ mp_raise_ValueError(MP_ERROR_TEXT("Invalid length of x2"));
+ }
+ if (y2_buf.len != 32) {
+ mp_raise_ValueError(MP_ERROR_TEXT("Invalid length of y2"));
+ }
+
// Convert coordinates to bignums
bn_read_be((uint8_t *)x1_buf.buf, &p1.x);
bn_read_be((uint8_t *)y1_buf.buf, &p1.y);
### ports/stm32/boards/Passport/modules/tests/test_unit.py
@@ -20,6 +20,10 @@ def test_error_codes(test):
assert test('error_codes.py') == b'OK'
+def test_ecdsa_bindings(test):
+ assert test('ecdsa_bindings.py') == b'OK'
+
+
def test_ext_settings(test):
assert test('ext_settings.py') == b'OK'
### ports/stm32/boards/Passport/modules/tests/unit/ecdsa_bindings.py
@@ -0,0 +1,61 @@
+# SPDX-FileCopyrightText: © 2026 Foundation Devices, Inc. <hello@foundation.xyz>
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+# Buffer length validation in the trezorcrypto.ecdsa bindings. bn_read_be()
+# always reads 32 bytes, so a shorter buffer would read past its end.
+
+from trezorcrypto import ecdsa
+from ubinascii import unhexlify as a2b_hex
+
+
+GENERATOR_X = a2b_hex('79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798')
+GENERATOR_Y = a2b_hex('483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8')
+
+BAD_LENGTHS = (
+ b'',
+ b'\x01',
+ b'\x01' * 31,
+ b'\x01' * 33,
+ b'\x01' * 64,
+)
+
+
+def scalar(value):
+ return bytes(31) + bytes([value])
+
+
+def must_reject(call):
+ try:
+ call()
+ except ValueError:
+ return
+
+ raise RuntimeError('expected ValueError')
+
+
+# Valid operations still work: 1 * G is the generator.
+x1, y1 = ecdsa.scalar_multiply(scalar(1))
+assert x1 == GENERATOR_X
+assert y1 == GENERATOR_Y
+
+# G + 2G == 3G, checked against scalar_multiply so the vector does not depend on
+# a hardcoded point. The two addends differ, so this is not the doubling case.
+x2, y2 = ecdsa.scalar_multiply(scalar(2))
+x3, y3 = ecdsa.scalar_multiply(scalar(3))
+assert ecdsa.point_add(x1, y1, x2, y2) == (x3, y3)
+
+# Every scalar length other than 32 is rejected.
+for bad in BAD_LENGTHS:
+ must_reject(lambda: ecdsa.scalar_multiply(bad))
+
+# ...and every coordinate of point_add, in each position.
+for bad in BAD_LENGTHS:
+ must_reject(lambda: ecdsa.point_add(bad, y1, x2, y2))
+ must_reject(lambda: ecdsa.point_add(x1, bad, x2, y2))
+ must_reject(lambda: ecdsa.point_add(x1, y1, bad, y2))
+ must_reject(lambda: ecdsa.point_add(x1, y1, x2, bad))
+
+# A rejected call must not have disturbed the valid path.
+assert ecdsa.point_add(x1, y1, x2, y2) == (x3, y3)
+
+return_value.write(b'OK')Why this scored 63/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.