From a106b57d0d5a9b721b21d43aac293bdb48a1dd98 Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Tue, 1 Sep 2026 18:09:50 +0300 Subject: [PATCH 1/2] Fix segfault in X509.set_serial_number for negative serials hex(serial) renders negative values as "-0x...", so slicing off the first two characters left a stray "x" and BN_hex2bn failed to parse it. The failure went undetected because the guard compared BN_hex2bn's int return value against _ffi.NULL, which is always unequal, so the NULL BIGNUM was passed straight to BN_to_ASN1_INTEGER and dereferenced. Format the hex digits directly, which BN_hex2bn accepts with a leading "-", and check its return value against 0 as documented. X509.get_serial_number already round-trips negative values. Co-authored-by: Claude --- CHANGELOG.rst | 2 ++ src/OpenSSL/crypto.py | 11 ++++++++--- tests/test_crypto.py | 13 +++++++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6175dcd3..47c37dd0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,6 +18,8 @@ Changes: - Fixed a race in which an exception raised by a verify, ALPN selection, OCSP, or DTLS cookie callback for one ``Connection`` could be raised on an unrelated ``Connection`` created from the same ``Context`` and used concurrently from another thread. Exceptions from these callbacks are now tracked per ``Connection``. Discovered and reported by SecDim Security Research. - Fixed exceptions raised by a verify callback registered with ``Connection.set_verify`` being swallowed instead of being propagated to the caller. +- Fixed a segfault in ``OpenSSL.crypto.X509.set_serial_number()`` when it was passed a negative serial number. + [`#1534 `_] 26.4.0 (2026-08-01) diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py index a7e8de69..8122ecc5 100644 --- a/src/OpenSSL/crypto.py +++ b/src/OpenSSL/crypto.py @@ -1004,14 +1004,19 @@ def set_serial_number(self, serial: int) -> None: if not isinstance(serial, int): raise TypeError("serial must be an integer") - hex_serial = hex(serial)[2:] + # hex(serial) renders negative values as "-0x...", so slicing off a + # two-character prefix would leave a stray "x". Format the digits + # directly instead; BN_hex2bn understands a leading "-". + hex_serial = f"{serial:x}" hex_serial_bytes = hex_serial.encode("ascii") bignum_serial = _ffi.new("BIGNUM**") - # BN_hex2bn stores the result in &bignum. + # BN_hex2bn stores the result in &bignum and returns the number of + # characters consumed, or 0 on error. It does not return a pointer, + # so it must not be compared against NULL. result = _lib.BN_hex2bn(bignum_serial, hex_serial_bytes) - _openssl_assert(result != _ffi.NULL) + _openssl_assert(result != 0) asn1_serial = _lib.BN_to_ASN1_INTEGER(bignum_serial[0], _ffi.NULL) _lib.BN_free(bignum_serial[0]) diff --git a/tests/test_crypto.py b/tests/test_crypto.py index 7508d1d0..37ec2857 100644 --- a/tests/test_crypto.py +++ b/tests/test_crypto.py @@ -1471,6 +1471,19 @@ def test_serial_number(self) -> None: certificate.set_serial_number(2**128 + 1) assert certificate.get_serial_number() == 2**128 + 1 + def test_negative_serial_number(self) -> None: + """ + `X509.set_serial_number` accepts a negative serial number, which + `X509.get_serial_number` then returns unchanged. + """ + certificate = X509() + certificate.set_serial_number(-1) + assert certificate.get_serial_number() == -1 + certificate.set_serial_number(-255) + assert certificate.get_serial_number() == -255 + certificate.set_serial_number(-(2**128 + 1)) + assert certificate.get_serial_number() == -(2**128 + 1) + def _setBoundTest( self, get: typing.Callable[[X509], bytes | None], From 6eba15863e0d0c1242e8fefd3960a534d0aee3d6 Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Tue, 1 Sep 2026 20:12:46 +0300 Subject: [PATCH 2/2] Reject negative serial numbers instead of accepting them RFC 5280 section 4.1.2.2 requires the serial number to be a positive integer, so set_serial_number now raises ValueError rather than encoding a value the standard does not permit. Moves the changelog entry to the backward-incompatible section: a negative serial previously crashed the interpreter, and callers relying on that value being stored will now see an exception. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo --- CHANGELOG.rst | 7 +++++-- src/OpenSSL/crypto.py | 11 +++++------ tests/test_crypto.py | 20 ++++++++++++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 47c37dd0..a5d78efe 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,11 @@ The third digit is only for regressions. Backward-incompatible changes: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +- ``OpenSSL.crypto.X509.set_serial_number()`` now raises ``ValueError`` for a negative serial number, which + `RFC 5280 section 4.1.2.2 `_ does not permit. + Passing one previously crashed the interpreter. + [`#1534 `_] + Deprecations: ^^^^^^^^^^^^^ @@ -18,8 +23,6 @@ Changes: - Fixed a race in which an exception raised by a verify, ALPN selection, OCSP, or DTLS cookie callback for one ``Connection`` could be raised on an unrelated ``Connection`` created from the same ``Context`` and used concurrently from another thread. Exceptions from these callbacks are now tracked per ``Connection``. Discovered and reported by SecDim Security Research. - Fixed exceptions raised by a verify callback registered with ``Connection.set_verify`` being swallowed instead of being propagated to the caller. -- Fixed a segfault in ``OpenSSL.crypto.X509.set_serial_number()`` when it was passed a negative serial number. - [`#1534 `_] 26.4.0 (2026-08-01) diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py index 8122ecc5..c4cfff3b 100644 --- a/src/OpenSSL/crypto.py +++ b/src/OpenSSL/crypto.py @@ -1004,17 +1004,16 @@ def set_serial_number(self, serial: int) -> None: if not isinstance(serial, int): raise TypeError("serial must be an integer") - # hex(serial) renders negative values as "-0x...", so slicing off a - # two-character prefix would leave a stray "x". Format the digits - # directly instead; BN_hex2bn understands a leading "-". + if serial < 0: + raise ValueError("serial must be non-negative") + hex_serial = f"{serial:x}" hex_serial_bytes = hex_serial.encode("ascii") bignum_serial = _ffi.new("BIGNUM**") - # BN_hex2bn stores the result in &bignum and returns the number of - # characters consumed, or 0 on error. It does not return a pointer, - # so it must not be compared against NULL. + # BN_hex2bn returns the count of characters consumed, or 0 on error -- + # not a pointer, so it must not be compared against NULL. result = _lib.BN_hex2bn(bignum_serial, hex_serial_bytes) _openssl_assert(result != 0) diff --git a/tests/test_crypto.py b/tests/test_crypto.py index 37ec2857..5db93427 100644 --- a/tests/test_crypto.py +++ b/tests/test_crypto.py @@ -1473,16 +1473,20 @@ def test_serial_number(self) -> None: def test_negative_serial_number(self) -> None: """ - `X509.set_serial_number` accepts a negative serial number, which - `X509.get_serial_number` then returns unchanged. + `X509.set_serial_number` rejects a negative serial number, which + RFC 5280 section 4.1.2.2 does not permit. Before this was checked, + a negative value crashed the interpreter. """ certificate = X509() - certificate.set_serial_number(-1) - assert certificate.get_serial_number() == -1 - certificate.set_serial_number(-255) - assert certificate.get_serial_number() == -255 - certificate.set_serial_number(-(2**128 + 1)) - assert certificate.get_serial_number() == -(2**128 + 1) + for serial in [-1, -255, -(2**128 + 1)]: + with pytest.raises(ValueError): + certificate.set_serial_number(serial) + + # A serial that was set earlier is left alone by the rejected call. + certificate.set_serial_number(1) + with pytest.raises(ValueError): + certificate.set_serial_number(-1) + assert certificate.get_serial_number() == 1 def _setBoundTest( self,