Fix segfault in X509.set_serial_number for negative serials

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 <https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.2>`_ does not permit.
+  Passing one previously crashed the interpreter.
+  [`#1534 <https://github.com/pyca/pyopenssl/pull/1534>`_]
+
 Deprecations:
 ^^^^^^^^^^^^^
 

src/OpenSSL/crypto.py

@@ -1004,14 +1004,18 @@ 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:]
+        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.
+        # 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 != _ffi.NULL)
+        _openssl_assert(result != 0)
 
         asn1_serial = _lib.BN_to_ASN1_INTEGER(bignum_serial[0], _ffi.NULL)
         _lib.BN_free(bignum_serial[0])

tests/test_crypto.py

@@ -1471,6 +1471,23 @@ 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` 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()
+        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,
         get: typing.Callable[[X509], bytes | None],