Skip to content

Commit

Permalink
fix a memory leak and a potential UAF and also pyca#722
Browse files Browse the repository at this point in the history
  • Loading branch information
reaperhulk committed Nov 29, 2017
1 parent f724786 commit 915d1d8
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 6 deletions.
6 changes: 4 additions & 2 deletions src/OpenSSL/SSL.py
Expand Up @@ -309,8 +309,10 @@ def __init__(self, callback):

@wraps(callback)
def wrapper(ok, store_ctx):
cert = X509.__new__(X509)
cert._x509 = _lib.X509_STORE_CTX_get_current_cert(store_ctx)
x509 = _lib.X509_STORE_CTX_get_current_cert(store_ctx)
res = _lib.X509_up_ref(x509)
_openssl_assert(res == 1)
cert = X509._from_raw_x509_ptr(x509)
error_number = _lib.X509_STORE_CTX_get_error(store_ctx)
error_depth = _lib.X509_STORE_CTX_get_error_depth(store_ctx)

Expand Down
7 changes: 3 additions & 4 deletions src/OpenSSL/crypto.py
Expand Up @@ -3058,8 +3058,7 @@ def load_pkcs12(buffer, passphrase=None):
pycert = None
friendlyname = None
else:
pycert = X509.__new__(X509)
pycert._x509 = _ffi.gc(cert[0], _lib.X509_free)
pycert = X509._from_raw_x509_ptr(cert[0])

friendlyname_length = _ffi.new("int*")
friendlyname_buffer = _lib.X509_alias_get0(
Expand All @@ -3073,8 +3072,8 @@ def load_pkcs12(buffer, passphrase=None):

pycacerts = []
for i in range(_lib.sk_X509_num(cacerts)):
pycacert = X509.__new__(X509)
pycacert._x509 = _lib.sk_X509_value(cacerts, i)
x509 = _lib.sk_X509_value(cacerts, i)
pycacert = X509._from_raw_x509_ptr(x509)
pycacerts.append(pycacert)
if not pycacerts:
pycacerts = None
Expand Down
25 changes: 25 additions & 0 deletions tests/test_ssl.py
Expand Up @@ -1279,6 +1279,31 @@ def callback(self, connection, *args):

assert verify.connection is clientConnection

def test_x509_in_verify_works(self):
"""
We had a bug where the X509 cert instantiated in the callback wrapper
didn't __init__ so it was missing objects needed when calling
get_subject. This test sets up a handshake where we call get_subject
on the cert provided to the verify callback.
"""
serverContext = Context(TLSv1_METHOD)
serverContext.use_privatekey(
load_privatekey(FILETYPE_PEM, cleartextPrivateKeyPEM))
serverContext.use_certificate(
load_certificate(FILETYPE_PEM, cleartextCertificatePEM))
serverConnection = Connection(serverContext, None)

def verify_cb_get_subject(conn, cert, errnum, depth, ok):
assert cert.get_subject()
return 1

clientContext = Context(TLSv1_METHOD)
clientContext.set_verify(VERIFY_PEER, verify_cb_get_subject)
clientConnection = Connection(clientContext, None)
clientConnection.set_connect_state()

handshake_in_memory(clientConnection, serverConnection)

def test_set_verify_callback_exception(self):
"""
If the verify callback passed to `Context.set_verify` raises an
Expand Down

0 comments on commit 915d1d8

Please sign in to comment.