Fixed two memory leaks in x509 csr extensions (#4434)

* Fixed a memory leak in x.509 OCSP no check

* Fix the _actual_ leak

* Speed up symbolizations

* Disable backtrace by default, because it doesn't work on Windows

* line length

4 files changed, 75 insertions, 6 deletions

diff --git a/src/_cffi_src/openssl/x509.py b/src/_cffi_src/openssl/x509.py
index 59fdbf7e4..3f2ac90dc 100644
--- a/src/_cffi_src/openssl/x509.py
+++ b/src/_cffi_src/openssl/x509.py
@@ -76,6 +76,8 @@ static const int XN_FLAG_FN_ALIGN;
 static const int XN_FLAG_RFC2253;
 static const int XN_FLAG_ONELINE;
 static const int XN_FLAG_MULTILINE;
+
+typedef void (*sk_X509_EXTENSION_freefunc)(X509_EXTENSION *);
 """
 
 FUNCTIONS = """
@@ -282,6 +284,7 @@ int sk_X509_EXTENSION_push(X509_EXTENSIONS *, X509_EXTENSION *);
 int sk_X509_EXTENSION_insert(X509_EXTENSIONS *, X509_EXTENSION *, int);
 X509_EXTENSION *sk_X509_EXTENSION_delete(X509_EXTENSIONS *, int);
 void sk_X509_EXTENSION_free(X509_EXTENSIONS *);
+void sk_X509_EXTENSION_pop_free(X509_EXTENSIONS *, sk_X509_EXTENSION_freefunc);
 
 int sk_X509_REVOKED_num(Cryptography_STACK_OF_X509_REVOKED *);
 X509_REVOKED *sk_X509_REVOKED_value(Cryptography_STACK_OF_X509_REVOKED *, int);
diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index bdf8f370f..cfd7c89fe 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -707,10 +707,15 @@ class Backend(object):
         sk_extension = self._lib.sk_X509_EXTENSION_new_null()
         self.openssl_assert(sk_extension != self._ffi.NULL)
         sk_extension = self._ffi.gc(
-            sk_extension, self._lib.sk_X509_EXTENSION_free
+            sk_extension,
+            lambda x: self._lib.sk_X509_EXTENSION_pop_free(
+                x, self._ffi.addressof(
+                    self._lib._original_lib, "X509_EXTENSION_free"
+                )
+            )
         )
-        # gc is not necessary for CSRs, as sk_X509_EXTENSION_free
-        # will release all the X509_EXTENSIONs.
+        # Don't GC individual extensions because the memory is owned by
+        # sk_extensions and will be freed along with it.
         self._create_x509_extensions(
             extensions=builder._extensions,
             handlers=_EXTENSION_ENCODE_HANDLERS,
diff --git a/src/cryptography/hazmat/backends/openssl/x509.py b/src/cryptography/hazmat/backends/openssl/x509.py
index b870eeb77..a7a2c70d9 100644
--- a/src/cryptography/hazmat/backends/openssl/x509.py
+++ b/src/cryptography/hazmat/backends/openssl/x509.py
@@ -429,6 +429,14 @@ class _CertificateSigningRequest(object):
     @utils.cached_property
     def extensions(self):
         x509_exts = self._backend._lib.X509_REQ_get_extensions(self._x509_req)
+        x509_exts = self._backend._ffi.gc(
+            x509_exts,
+            lambda x: self._backend._lib.sk_X509_EXTENSION_pop_free(
+                x, self._backend._ffi.addressof(
+                    self._backend._lib._original_lib, "X509_EXTENSION_free"
+                )
+            )
+        )
         return _CSR_EXTENSION_PARSER.parse(self._backend, x509_exts)
 
     def public_bytes(self, encoding):
diff --git a/tests/hazmat/backends/test_openssl_memleak.py b/tests/hazmat/backends/test_openssl_memleak.py
index 5cb7cbc77..34ad11ba4 100644
--- a/tests/hazmat/backends/test_openssl_memleak.py
+++ b/tests/hazmat/backends/test_openssl_memleak.py
@@ -23,14 +23,46 @@ def main(argv):
     import gc
     import json
 
+    import cffi
+
     from cryptography.hazmat.bindings._openssl import ffi, lib
 
     heap = {}
 
+    BACKTRACE_ENABLED = False
+    if BACKTRACE_ENABLED:
+        backtrace_ffi = cffi.FFI()
+        backtrace_ffi.cdef('''
+            int backtrace(void **, int);
+            char **backtrace_symbols(void *const *, int);
+        ''')
+        backtrace_lib = backtrace_ffi.dlopen(None)
+
+        def backtrace():
+            buf = backtrace_ffi.new("void*[]", 24)
+            length = backtrace_lib.backtrace(buf, len(buf))
+            return (buf, length)
+
+        def symbolize_backtrace(trace):
+            (buf, length) = trace
+            symbols = backtrace_lib.backtrace_symbols(buf, length)
+            stack = [
+                backtrace_ffi.string(symbols[i]).decode()
+                for i in range(length)
+            ]
+            lib.Cryptography_free_wrapper(symbols, backtrace_ffi.NULL, 0)
+            return stack
+    else:
+        def backtrace():
+            return None
+
+        def symbolize_backtrace(trace):
+            return None
+
     @ffi.callback("void *(size_t, const char *, int)")
     def malloc(size, path, line):
         ptr = lib.Cryptography_malloc_wrapper(size, path, line)
-        heap[ptr] = (size, path, line)
+        heap[ptr] = (size, path, line, backtrace())
         return ptr
 
     @ffi.callback("void *(void *, size_t, const char *, int)")
@@ -38,7 +70,7 @@ def main(argv):
         if ptr != ffi.NULL:
             del heap[ptr]
         new_ptr = lib.Cryptography_realloc_wrapper(ptr, size, path, line)
-        heap[new_ptr] = (size, path, line)
+        heap[new_ptr] = (size, path, line, backtrace())
         return new_ptr
 
     @ffi.callback("void(void *, const char *, int)")
@@ -81,6 +113,7 @@ def main(argv):
                 "size": heap[ptr][0],
                 "path": ffi.string(heap[ptr][1]).decode(),
                 "line": heap[ptr][2],
+                "backtrace": symbolize_backtrace(heap[ptr][3]),
             })
             for ptr in remaining
         )))
@@ -177,7 +210,7 @@ class TestOpenSSLMemoryLeaks(object):
     @pytest.mark.parametrize("path", [
         "x509/PKITS_data/certs/ValidcRLIssuerTest28EE.crt",
     ])
-    def test_x509_extensions(self, path):
+    def test_x509_certificate_extensions(self, path):
         assert_no_memory_leaks(textwrap.dedent("""
         def func(path):
             from cryptography import x509
@@ -193,6 +226,26 @@ class TestOpenSSLMemoryLeaks(object):
             cert.extensions
         """), [path])
 
+    def test_x509_csr_extensions(self):
+        assert_no_memory_leaks(textwrap.dedent("""
+        def func():
+            from cryptography import x509
+            from cryptography.hazmat.backends.openssl import backend
+            from cryptography.hazmat.primitives import hashes
+            from cryptography.hazmat.primitives.asymmetric import rsa
+
+            private_key = rsa.generate_private_key(
+                key_size=2048, public_exponent=65537, backend=backend
+            )
+            cert = x509.CertificateSigningRequestBuilder().subject_name(
+                x509.Name([])
+            ).add_extension(
+               x509.OCSPNoCheck(), critical=False
+            ).sign(private_key, hashes.SHA256(), backend)
+
+            cert.extensions
+        """))
+
     def test_ec_private_numbers_private_key(self):
         assert_no_memory_leaks(textwrap.dedent("""
         def func():