Fix a variety of crashes on PyPy or on various builds of OpenSSL

5 files changed, 119 insertions, 16 deletions

diff --git a/OpenSSL/crypto/crl.c b/OpenSSL/crypto/crl.c
index 614a606..3f56d83 100644
--- a/OpenSSL/crypto/crl.c
+++ b/OpenSSL/crypto/crl.c
@@ -2,7 +2,6 @@
 #define crypto_MODULE
 #include "crypto.h"
 
-
 static X509_REVOKED * X509_REVOKED_dup(X509_REVOKED *orig) {
     X509_REVOKED *dupe = NULL;
 
@@ -138,6 +137,20 @@ crypto_CRL_export(crypto_CRLObj *self, PyObject *args, PyObject *keywds) {
         return NULL;
     }
 
+
+#if OPENSSL_VERSION_NUMBER >= 0x01000000L
+    /* Older versions of OpenSSL had no problem with trying to export using an
+     * uninitialized key.  Newer versions segfault, instead.  We can only check
+     * on the new versions, though, because the old versions don't even have the
+     * field that the segfault is triggered by.
+     */
+    if (!key->pkey->ameth) {
+        PyErr_SetString(
+            crypto_Error, "Cannot export with an unitialized key");
+        return NULL;
+    }
+#endif
+
     bio = BIO_new(BIO_s_mem());
     tmptm = ASN1_TIME_new();
     if (!tmptm) {
diff --git a/OpenSSL/ssl/connection.c b/OpenSSL/ssl/connection.c
index ebbe39f..9428376 100755
--- a/OpenSSL/ssl/connection.c
+++ b/OpenSSL/ssl/connection.c
@@ -425,17 +425,98 @@ ssl_Connection_send(ssl_ConnectionObj *self, PyObject *args) {
     char *buf;
 
 #if PY_VERSION_HEX >= 0x02060000
-    Py_buffer pbuf;
 
-    if (!PyArg_ParseTuple(args, "s*|i:send", &pbuf, &flags))
+    /*
+     * Sit back and I'll tell you a story of intrigue and corruption, deceit and
+     * murder.
+     *
+     * A Py_buffer is used to hold any kind of byte-like data - a string, a
+     * memoryview, a buffer, etc.  PyArg_ParseTuple takes whatever kind of
+     * object was supplied, notices the "s*" format specifier, and tries to copy
+     * the metadata for that object into the Py_buffer also passed in.
+     *
+     * According to the Python documentation:
+     *
+     *     ... the caller is responsible for calling PyBuffer_Release with the
+     *     structure after it has processed the data.
+     *
+     * Correct use of PyBuffer_Release is necessary due to the fact that
+     * Py_buffer must hold a reference to the original Python object from which
+     * it was initialized.  PyBuffer_Release will decrement the reference count
+     * of that original object, allowing it to eventually be deallocated - but
+     * only after the Py_buffer is no longer in use.
+     *
+     * To support failures partway through parsing a format string,
+     * PyArg_ParseTuple maintains an internal PyListObject of PyCObjects it has
+     * created so far.  This allows it to easily clean up these objects when
+     * parsing fails, before returning an error to the caller (incidentally, it
+     * also makes sure to clean up the Py_buffer it initialized in this case, by
+     * calling PyBuffer_Release - which means the caller *must not* use
+     * PyBuffer_Release when PyArg_ParseTuple fails; not exactly what the
+     * documentation directs).
+     *
+     * The PyCObjects are given destructors which clean up some structure
+     * PyArg_ParseTuple has created (or initialized) - often another PyObject
+     * which needs to be decref'd.
+     *
+     * When parsing completes, the reference count of the PyListObject is merely
+     * decremented.  The normal Python garbage collection logic (the reference
+     * counting logic, in this case) takes over and collects both the list and
+     * all of the objects in it.  When each PyCObject in the list is collected,
+     * it triggers its destructor to clean up the structure it wraps.  This all
+     * happens immediately, before the Py_XDECREF of the PyListObject returns.
+     *
+     * The PyListObject is similarly destroyed in the success case, but not
+     * until each PyCObject it contains has had its destructor set to NULL to
+     * prevent it from cleaning up its contents.
+     *
+     * When PyArg_ParseTuple returns in an error case, therefore,
+     * PyRelease_Buffer has already been used on the Py_buffer passed to
+     * PyArg_ParseTuple.
+     *
+     * This is fortuitous, as the Py_buffer is typically (always?) allocated on
+     * the stack of the caller of PyArg_ParseTuple.  Once that caller returns,
+     * that stack memory is no longer valid, and the Py_buffer may no longer be
+     * used.
+     *
+     * On a Python runtime which does not use reference counting, the
+     * PyListObject may not actually be collected before Py_XDECREF returns.  It
+     * may not even be collected before PyArg_ParseTuple returns.  In fact, in
+     * particularly unfortunate cases, it may even not be collected before the
+     * caller of PyArg_ParseTuple returns.  It may not be called until long,
+     * long after the stack memory the Py_buffer was allocated on has been
+     * re-used for some other function call, after the memory holding a pointer
+     * to the structure that owns the memory the Py_buffer wrapped has been
+     * overwritten.  When PyBuffer_Release is used on the Py_buffer in this
+     * case, it will try to decrement a random integer - probably part of a
+     * local variable on some part of the stack.
+     *
+     * The PyPy runtime does not use reference counting.
+     *
+     * The solution adopted here is to allocate the Py_buffer on the heap,
+     * instead.  As there is no mechanism for learning when the PyCObject used
+     * by PyArg_ParseTuple to do its internal cleanup has had its way with the
+     * Py_buffer, the Py_buffer is leaked in the error case, to ensure it is
+     * still valid whenever PyBuffer_Release is called on it.
+     *
+     * Real programs should rarely, if ever, trigger the error case of
+     * PyArg_ParseTuple, so this is probably okay.  Plus, I'm tired of this
+     * stupid bug. -exarkun
+     */
+
+    Py_buffer *pbuf = PyMem_Malloc(sizeof *pbuf);
+
+    if (!PyArg_ParseTuple(args, "s*|i:send", pbuf, &flags)) {
         return NULL;
+    }
 
-    buf = pbuf.buf;
-    len = pbuf.len;
+    buf = pbuf->buf;
+    len = pbuf->len;
 #else
 
-    if (!PyArg_ParseTuple(args, "s#|i:send", &buf, &len, &flags))
+    if (!PyArg_ParseTuple(args, "s#|i:send", &buf, &len, &flags)) {
         return NULL;
+    }
 #endif
 
     MY_BEGIN_ALLOW_THREADS(self->tstate)
@@ -443,7 +524,8 @@ ssl_Connection_send(ssl_ConnectionObj *self, PyObject *args) {
     MY_END_ALLOW_THREADS(self->tstate)
 
 #if PY_VERSION_HEX >= 0x02060000
-    PyBuffer_Release(&pbuf);
+    PyBuffer_Release(pbuf);
+    PyMem_Free(pbuf);
 #endif
 
     if (PyErr_Occurred())
@@ -482,16 +564,18 @@ ssl_Connection_sendall(ssl_ConnectionObj *self, PyObject *args)
     PyObject *pyret = Py_None;
 
 #if PY_VERSION_HEX >= 0x02060000
-    Py_buffer pbuf;
+    Py_buffer *pbuf = PyMem_Malloc(sizeof *pbuf);
 
-    if (!PyArg_ParseTuple(args, "s*|i:sendall", &pbuf, &flags))
+    if (!PyArg_ParseTuple(args, "s*|i:sendall", pbuf, &flags)) {
         return NULL;
+    }
 
-    buf = pbuf.buf;
-    len = pbuf.len;
+    buf = pbuf->buf;
+    len = pbuf->len;
 #else
-    if (!PyArg_ParseTuple(args, "s#|i:sendall", &buf, &len, &flags))
+    if (!PyArg_ParseTuple(args, "s#|i:sendall", &buf, &len, &flags)) {
         return NULL;
+    }
 #endif
 
     do {
@@ -520,7 +604,8 @@ ssl_Connection_sendall(ssl_ConnectionObj *self, PyObject *args)
     } while (len > 0);
 
 #if PY_VERSION_HEX >= 0x02060000
-    PyBuffer_Release(&pbuf);
+    PyBuffer_Release(pbuf);
+    PyMem_Free(pbuf);
 #endif
 
     Py_XINCREF(pyret);
diff --git a/OpenSSL/ssl/context.c b/OpenSSL/ssl/context.c
index 0b9f4b6..e971c0a 100644
--- a/OpenSSL/ssl/context.c
+++ b/OpenSSL/ssl/context.c
@@ -1249,7 +1249,7 @@ ssl_Context_init(ssl_ContextObj *self, int i_method) {
 #ifdef OPENSSL_NO_SSL2
             PyErr_SetString(PyExc_ValueError, "SSLv2_METHOD not supported by this version of OpenSSL");
             return NULL;
-#else      
+#else
             method = SSLv2_method();
 #endif
             break;
@@ -1268,6 +1268,11 @@ ssl_Context_init(ssl_ContextObj *self, int i_method) {
     }
 
     self->ctx = SSL_CTX_new(method);
+    if (self->ctx == NULL) {
+        exception_from_error_queue(ssl_Error);
+        return NULL;
+    }
+
     Py_INCREF(Py_None);
     self->passphrase_callback = Py_None;
     Py_INCREF(Py_None);
diff --git a/OpenSSL/test/test_crypto.py b/OpenSSL/test/test_crypto.py
index 62b9429..16799ef 100644
--- a/OpenSSL/test/test_crypto.py
+++ b/OpenSSL/test/test_crypto.py
@@ -2603,7 +2603,7 @@ class CRLTests(TestCase):
     def test_export_invalid(self):
         """
         If :py:obj:`CRL.export` is used with an uninitialized :py:obj:`X509`
-        instance, :py:obj:`ValueError` is raised.
+        instance, :py:obj:`OpenSSL.crypto.Error` is raised.
         """
         crl = CRL()
         self.assertRaises(Error, crl.export, X509(), PKey())
diff --git a/OpenSSL/test/test_ssl.py b/OpenSSL/test/test_ssl.py
index 721c1c0..3e4e3da 100644
--- a/OpenSSL/test/test_ssl.py
+++ b/OpenSSL/test/test_ssl.py
@@ -313,7 +313,7 @@ class ContextTests(TestCase, _LoopbackMixin):
 
         try:
             Context(SSLv2_METHOD)
-        except ValueError:
+        except (Error, ValueError):
             # Some versions of OpenSSL have SSLv2, some don't.
             # Difficult to say in advance.
             pass