← Watch feed
chore(core): clean python der implementation
Public commit record
57/100 · Thin
What the developer wrote
Authored by M1nd3r
chore(core): clean python der implementation
[no changelog]
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope! No meaningful explanatory body
What changed, and why it matters
This commit refactors the way digital signatures are encoded and decoded in the Trezor firmware's Python code. It replaces a generic low-level DER sequence encoder/decoder with two dedicated functions for signatures. The change is described by the developer as a cleanup ('chore') with no changelog entry. There is no direct evidence in the commit that this fixes a security vulnerability, but centralizing signature handling can reduce the risk of future mistakes.
Recommended action
Treat as a hardening/cleanup change rather than an urgent security fix. Review the new helpers for correct DER handling of edge cases (e.g., all-zero or high-bit integers) and ensure all callers have been migrated. Run the updated unit tests and consider adding negative test cases for malformed DER input.
Security signals we found
01
Refactoring of cryptographic signature encoding/decoding
02
Addition of input-length validation in `encode_signature()` (64/65 bytes) and integer-size validation in `decode_signature()` (<=32 bytes)
03
Removal of public generic DER sequence API in favor of signature-specific API
04
No changelog entry and commit tagged as chore
Evidence from the diff
The patch consolidates DER encoding/decoding for ECDSA signatures into encode_signature() and decode_signature() in core/src/trezor/crypto/der.py. Callers in Bitcoin, Ripple, and WebAuthn/FIDO2 code are updated to use these helpers instead of manually slicing signature components and calling encode_seq()/decode_seq(). The new helpers enforce that input signatures are 64 or 65 bytes and that decoded DER integers fit in 32 bytes, returning a fixed 64-byte raw signature. The old public encode_seq() and decode_seq() are removed or made private (_encode_int_seq, _decode_int_seq). Tests are updated accordingly.
Changed components
core/src/trezor/crypto/der.pycore/src/apps/bitcoin/common.pycore/src/apps/bitcoin/verification.pycore/src/apps/ripple/sign_tx.pycore/src/apps/webauthn/credential.pycore/src/apps/webauthn/fido2.pycore/tests/test_trezor.crypto.der.pyInspect captured patch +82 / −68
diff --git a/core/src/apps/bitcoin/common.py b/core/src/apps/bitcoin/common.py
index 95e65b60..a0bd8972 100644
--- a/core/src/apps/bitcoin/common.py
+++ b/core/src/apps/bitcoin/common.py
@@ -111,7 +111,7 @@ def ecdsa_sign(node: bip32.HDNode, digest: bytes) -> AnyBytes:
from trezor.crypto.curve import secp256k1
sig = secp256k1.sign(node.private_key(), digest)
- sigder = der.encode_seq((sig[1:33], sig[33:65]))
+ sigder = der.encode_signature(sig)
return sigder
diff --git a/core/src/apps/bitcoin/verification.py b/core/src/apps/bitcoin/verification.py
index f7c70ce9..557c11a9 100644
--- a/core/src/apps/bitcoin/verification.py
+++ b/core/src/apps/bitcoin/verification.py
@@ -136,27 +136,14 @@ class SignatureVerifier:
raise DataError("Invalid signature")
def verify_ecdsa(self, digest: bytes) -> None:
+ from trezor.crypto import der
from trezor.crypto.curve import secp256k1
try:
i = 0
for der_signature, _ in self.signatures:
- signature = _decode_der_signature(der_signature)
+ signature = der.decode_signature(der_signature)
while not secp256k1.verify(self.public_keys[i], signature, digest):
i += 1
except Exception:
raise DataError("Invalid signature")
-
-
-def _decode_der_signature(der_signature: memoryview) -> bytearray:
- from trezor.crypto import der
-
- seq = der.decode_seq(der_signature)
- if len(seq) != 2 or any(len(i) > 32 for i in seq):
- raise ValueError
-
- signature = bytearray(64)
- signature[32 - len(seq[0]) : 32] = seq[0]
- signature[64 - len(seq[1]) : 64] = seq[1]
-
- return signature
diff --git a/core/src/apps/ripple/sign_tx.py b/core/src/apps/ripple/sign_tx.py
index aaa30e04..d332f13f 100644
--- a/core/src/apps/ripple/sign_tx.py
+++ b/core/src/apps/ripple/sign_tx.py
@@ -77,7 +77,7 @@ async def sign_tx(msg: RippleSignTx, keychain: Keychain) -> RippleSignedTx:
# Signs and encodes signature into DER format
first_half_of_sha512 = sha512(to_sign).digest()[:32]
sig = secp256k1.sign(node.private_key(), first_half_of_sha512)
- sig_encoded = der.encode_seq((sig[1:33], sig[33:65]))
+ sig_encoded = der.encode_signature(sig)
tx = serialize(msg, source_address, node.public_key(), sig_encoded)
show_continue_in_app(TR.send__transaction_signed)
diff --git a/core/src/apps/webauthn/credential.py b/core/src/apps/webauthn/credential.py
index beb53f81..5dc941a0 100644
--- a/core/src/apps/webauthn/credential.py
+++ b/core/src/apps/webauthn/credential.py
@@ -96,7 +96,7 @@ class Credential:
for segment in data:
dig.update(segment)
sig = nist256p1.sign(self._private_key(), dig.digest(), False)
- return der.encode_seq((sig[1:33], sig[33:]))
+ return der.encode_signature(sig)
def bogus_signature(self) -> bytes:
raise NotImplementedError
@@ -351,7 +351,7 @@ class Fido2Credential(Credential):
COSE_ALG_ES256,
COSE_CURVE_P256,
):
- return der.encode_seq((b"\x0a" * 32, b"\x0a" * 32))
+ return der.encode_signature(b"\x0a" * 64)
elif (self.algorithm, self.curve) == (
COSE_ALG_EDDSA,
COSE_CURVE_ED25519,
@@ -403,7 +403,7 @@ class U2fCredential(Credential):
return self._u2f_sign(data)
def bogus_signature(self) -> AnyBytes:
- return der.encode_seq((b"\x0a" * 32, b"\x0a" * 32))
+ return der.encode_signature(b"\x0a" * 64)
def generate_key_handle(self) -> None:
# derivation path is m/U2F'/r'/r'/r'/r'/r'/r'/r'/r'
diff --git a/core/src/apps/webauthn/fido2.py b/core/src/apps/webauthn/fido2.py
index c6ec0baf..74be5db5 100644
--- a/core/src/apps/webauthn/fido2.py
+++ b/core/src/apps/webauthn/fido2.py
@@ -1311,7 +1311,7 @@ def basic_attestation_sign(data: Iterable[AnyBytes]) -> AnyBytes:
for segment in data:
dig.update(segment)
sig = nist256p1.sign(_FIDO_ATT_PRIV_KEY, dig.digest(), False)
- return der.encode_seq((sig[1:33], sig[33:]))
+ return der.encode_signature(sig)
def _msg_register_sign(challenge: bytes, cred: U2fCredential) -> bytes:
diff --git a/core/src/trezor/crypto/der.py b/core/src/trezor/crypto/der.py
index 60f21d49..b0ff4fa4 100644
--- a/core/src/trezor/crypto/der.py
+++ b/core/src/trezor/crypto/der.py
@@ -9,16 +9,32 @@ if TYPE_CHECKING:
# Maximum length of a DER-encoded secp256k1 or secp256p1 signature.
_MAX_DER_SIGNATURE_LENGTH = const(72)
+_DER_TAG_SEQUENCE = const(0x30)
+_DER_TAG_INTEGER = const(0x02)
+
+
+def decode_signature(der_signature: AnyBytes) -> bytearray:
+ seq = _decode_int_seq(der_signature)
+ if len(seq) != 2 or any(len(i) > 32 for i in seq):
+ raise ValueError # invalid or unsupported signature
+
+ signature = bytearray(64)
+ signature[32 - len(seq[0]) : 32] = seq[0]
+ signature[64 - len(seq[1]) : 64] = seq[1]
+
+ return signature
-def encode_length(l: int) -> bytes:
- if l < 0x80:
- return bytes([l])
- elif l <= 0xFF:
- return bytes([0x81, l])
- elif l <= 0xFFFF:
- return bytes([0x82, l >> 8, l & 0xFF])
- else:
- raise ValueError
+
+def encode_signature(signature: AnyBytes) -> AnyBytes:
+
+ if len(signature) not in (64, 65):
+ raise ValueError # invalid or unsupported signature
+
+ offset = 1 if len(signature) == 65 else 0
+ r = signature[offset : offset + 32]
+ s = signature[offset + 32 : offset + 64]
+
+ return _encode_int_seq(r, s)
def read_length(r: BufferReader) -> int:
@@ -41,26 +57,21 @@ def read_length(r: BufferReader) -> int:
return n
-def _write_int(w: Writer, number: AnyBytes) -> None:
- i = 0
- while i < len(number) and number[i] == 0:
- i += 1
-
- length = len(number) - i
- w.append(0x02)
- if length == 0 or number[i] >= 0x80:
- w.extend(encode_length(length + 1))
- w.append(0x00)
+def _encode_length(len: int) -> bytes:
+ if len < 0x80:
+ return bytes([len])
+ elif len <= 0xFF:
+ return bytes([0x81, len])
+ elif len <= 0xFFFF:
+ return bytes([0x82, len >> 8, len & 0xFF])
else:
- w.extend(encode_length(length))
-
- w.extend(memoryview(number)[i:])
+ raise ValueError
def _read_int(r: BufferReader) -> memoryview:
peek = r.peek # local_cache_attribute
- if r.get() != 0x02:
+ if r.get() != _DER_TAG_INTEGER:
raise ValueError
n = read_length(r)
@@ -82,24 +93,28 @@ def _read_int(r: BufferReader) -> memoryview:
return r.read_memoryview(n)
-def encode_seq(seq: tuple[AnyBytes, ...]) -> AnyBytes:
- from trezor.utils import empty_bytearray
+def _write_int(w: Writer, number: AnyBytes) -> None:
+ i = 0
+ while i < len(number) and number[i] == 0:
+ i += 1
- # Preallocate space for a signature, which is all that this function ever encodes.
- buffer = empty_bytearray(_MAX_DER_SIGNATURE_LENGTH)
- buffer.append(0x30)
- for i in seq:
- _write_int(buffer, i)
- buffer[1:1] = encode_length(len(buffer) - 1)
- return buffer
+ length = len(number) - i
+ w.append(_DER_TAG_INTEGER)
+ if length == 0 or number[i] >= 0x80:
+ w.extend(_encode_length(length + 1))
+ w.append(0x00)
+ else:
+ w.extend(_encode_length(length))
+
+ w.extend(memoryview(number)[i:])
-def decode_seq(data: memoryview) -> list[memoryview]:
+def _decode_int_seq(data: AnyBytes) -> list[memoryview]:
from trezor.utils import BufferReader
r = BufferReader(data)
- if r.get() != 0x30:
+ if r.get() != _DER_TAG_SEQUENCE:
raise ValueError
n = read_length(r)
@@ -113,3 +128,16 @@ def decode_seq(data: memoryview) -> list[memoryview]:
raise ValueError
return seq
+
+
+def _encode_int_seq(
+ *seq: AnyBytes, buffer_preallocate_len: int = _MAX_DER_SIGNATURE_LENGTH
+) -> AnyBytes:
+ from trezor.utils import empty_bytearray
+
+ buffer = empty_bytearray(buffer_preallocate_len)
+ buffer.append(_DER_TAG_SEQUENCE)
+ for i in seq:
+ _write_int(buffer, i)
+ buffer[1:1] = _encode_length(len(buffer) - 1)
+ return buffer
diff --git a/core/tests/test_trezor.crypto.der.py b/core/tests/test_trezor.crypto.der.py
index cad1431e..948a3009 100644
--- a/core/tests/test_trezor.crypto.der.py
+++ b/core/tests/test_trezor.crypto.der.py
@@ -6,7 +6,7 @@ from trezor.crypto import der
class TestCryptoDer(unittest.TestCase):
- vectors_seq = [
+ vectors_sig = (
(
(
"9a0b7be0d4ed3146ee262b42202841834698bb3ee39c24e7437df208b8b70771",
@@ -70,30 +70,29 @@ class TestCryptoDer(unittest.TestCase):
),
"3008020200ee020200ff",
),
- ]
+ )
- def test_der_encode_seq(self):
+ def test_der_encode_decode_signature(self):
- for s, d in self.vectors_seq:
+ for s, d in self.vectors_sig:
s = tuple(unhexlify(i) for i in s)
d = unhexlify(d)
- d2 = der.encode_seq(s)
+ sig = b"".join(s)
+ d2 = der.encode_signature(sig)
self.assertEqual(d2, d)
- s = [i.lstrip(b"\x00") for i in s]
- s2 = der.decode_seq(d)
- self.assertEqual(s2, s)
+ s2 = der.decode_signature(d)
+ self.assertEqual(s2, sig)
def test_der_encode_decode_long_seq(self):
for length in (1, 127, 128, 129, 255, 256, 257):
raw_int = bytes((i & 0xFE) + 1 for i in range(length))
for leading_zeros in range(3):
- encoded = der.encode_seq((b"\x00" * leading_zeros + raw_int,))
- decoded = der.decode_seq(encoded)
+ encoded = der._encode_int_seq(b"\x00" * leading_zeros + raw_int)
+ decoded = der._decode_int_seq(encoded)
self.assertEqual(decoded, [raw_int])
-
for zeroes in range(3):
- encoded = der.encode_seq((b"\x00" * zeroes,))
- decoded = der.decode_seq(encoded)
+ encoded = der._encode_int_seq(b"\x00" * zeroes)
+ decoded = der._decode_int_seq(encoded)
self.assertEqual(decoded, [b"\x00"])
Risk score
Our methodology →Why this scored 29/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.