From 4aa52c33d3ee51c632e0e1e10cafb7745fd1028c Mon Sep 17 00:00:00 2001
From: Alex Gaynor <alex.gaynor@gmail.com>
Date: Mon, 20 Nov 2017 09:04:08 -0500
Subject: [PATCH] Don't use things after they're freed...duh (#709)

* Don't use things after they're freed...duh

* changelog

* more details
---
 CHANGELOG.rst         |  3 ++-
 src/OpenSSL/SSL.py    |  7 ++-----
 src/OpenSSL/crypto.py | 45 ++++++++++++++++++++++++++++++++++---------
 3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 5025cc864..0eb7f815a 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -23,8 +23,9 @@ Deprecations:
 Changes:
 ^^^^^^^^
 
-*none*
 
+- Corrected a use-after-free when reusing an issuer or subject from an ``X509`` object after the underlying object has been mutated.
+  `#709 <https://github.com/pyca/pyopenssl/pull/709>`_
 
 ----
 
diff --git a/src/OpenSSL/SSL.py b/src/OpenSSL/SSL.py
index 667131b5f..39b7bdcb9 100644
--- a/src/OpenSSL/SSL.py
+++ b/src/OpenSSL/SSL.py
@@ -1957,9 +1957,7 @@ def get_peer_certificate(self):
         """
         cert = _lib.SSL_get_peer_certificate(self._ssl)
         if cert != _ffi.NULL:
-            pycert = X509.__new__(X509)
-            pycert._x509 = _ffi.gc(cert, _lib.X509_free)
-            return pycert
+            return X509._from_raw_x509_ptr(cert)
         return None
 
     def get_peer_cert_chain(self):
@@ -1977,8 +1975,7 @@ def get_peer_cert_chain(self):
         for i in range(_lib.sk_X509_num(cert_stack)):
             # TODO could incref instead of dup here
             cert = _lib.X509_dup(_lib.sk_X509_value(cert_stack, i))
-            pycert = X509.__new__(X509)
-            pycert._x509 = _ffi.gc(cert, _lib.X509_free)
+            pycert = X509._from_raw_x509_ptr(cert)
             result.append(pycert)
         return result
 
diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py
index 72c04b0f2..ee422cbaa 100644
--- a/src/OpenSSL/crypto.py
+++ b/src/OpenSSL/crypto.py
@@ -162,6 +162,19 @@ def _get_asn1_time(timestamp):
             return string_result
 
 
+class _X509NameInvalidator(object):
+    def __init__(self):
+        self._names = []
+
+    def add(self, name):
+        self._names.append(name)
+
+    def clear(self):
+        for name in self._names:
+            # Breaks the object, but also prevents UAF!
+            del name._name
+
+
 class PKey(object):
     """
     A class representing an DSA or RSA public key or key pair.
@@ -1032,6 +1045,17 @@ def __init__(self):
         _openssl_assert(x509 != _ffi.NULL)
         self._x509 = _ffi.gc(x509, _lib.X509_free)
 
+        self._issuer_invalidator = _X509NameInvalidator()
+        self._subject_invalidator = _X509NameInvalidator()
+
+    @classmethod
+    def _from_raw_x509_ptr(cls, x509):
+        cert = cls.__new__(cls)
+        cert._x509 = _ffi.gc(x509, _lib.X509_free)
+        cert._issuer_invalidator = _X509NameInvalidator()
+        cert._subject_invalidator = _X509NameInvalidator()
+        return cert
+
     def to_cryptography(self):
         """
         Export as a ``cryptography`` certificate.
@@ -1382,7 +1406,9 @@ def get_issuer(self):
         :return: The issuer of this certificate.
         :rtype: :class:`X509Name`
         """
-        return self._get_name(_lib.X509_get_issuer_name)
+        name = self._get_name(_lib.X509_get_issuer_name)
+        self._issuer_invalidator.add(name)
+        return name
 
     def set_issuer(self, issuer):
         """
@@ -1393,7 +1419,8 @@ def set_issuer(self, issuer):
 
         :return: ``None``
         """
-        return self._set_name(_lib.X509_set_issuer_name, issuer)
+        self._set_name(_lib.X509_set_issuer_name, issuer)
+        self._issuer_invalidator.clear()
 
     def get_subject(self):
         """
@@ -1407,7 +1434,9 @@ def get_subject(self):
         :return: The subject of this certificate.
         :rtype: :class:`X509Name`
         """
-        return self._get_name(_lib.X509_get_subject_name)
+        name = self._get_name(_lib.X509_get_subject_name)
+        self._subject_invalidator.add(name)
+        return name
 
     def set_subject(self, subject):
         """
@@ -1418,7 +1447,8 @@ def set_subject(self, subject):
 
         :return: ``None``
         """
-        return self._set_name(_lib.X509_set_subject_name, subject)
+        self._set_name(_lib.X509_set_subject_name, subject)
+        self._subject_invalidator.clear()
 
     def get_extension_count(self):
         """
@@ -1691,8 +1721,7 @@ def _exception_from_context(self):
         # expect this call to never return :class:`None`.
         _x509 = _lib.X509_STORE_CTX_get_current_cert(self._store_ctx)
         _cert = _lib.X509_dup(_x509)
-        pycert = X509.__new__(X509)
-        pycert._x509 = _ffi.gc(_cert, _lib.X509_free)
+        pycert = X509._from_raw_x509_ptr(_cert)
         return X509StoreContextError(errors, pycert)
 
     def set_store(self, store):
@@ -1755,9 +1784,7 @@ def load_certificate(type, buffer):
     if x509 == _ffi.NULL:
         _raise_current_error()
 
-    cert = X509.__new__(X509)
-    cert._x509 = _ffi.gc(x509, _lib.X509_free)
-    return cert
+    return X509._from_raw_x509_ptr(x509)
 
 
 def dump_certificate(type, cert):