From 463db85110658db729c722e6f2ef63fc67b4788b Mon Sep 17 00:00:00 2001
From: Sebastian Thiel <byronimo@gmail.com>
Date: Tue, 7 Mar 2017 07:39:25 +0100
Subject: [PATCH] Don't allow Stacks to be allocated with a null-ptr

The latter must be seen as undefined behaviour, as it will cause
the `sk_num` function to return -1 to indicate the error, which
causes all kinds of issues.

Thus there now is a panic to abort the program if stacks are initialized
with a null-ptr, and special handling of that case when decoding
a Pkcs file.
---
 openssl/src/pkcs12.rs |  9 +++++++--
 openssl/src/stack.rs  | 15 ++++-----------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/openssl/src/pkcs12.rs b/openssl/src/pkcs12.rs
index 31864802..31aae536 100644
--- a/openssl/src/pkcs12.rs
+++ b/openssl/src/pkcs12.rs
@@ -42,7 +42,12 @@ impl Pkcs12Ref {
 
             let pkey = PKey::from_ptr(pkey);
             let cert = X509::from_ptr(cert);
-            let chain = Stack::from_ptr(chain);
+
+            let chain = if chain.is_null() {
+                try!(Stack::new())
+            } else {
+                Stack::from_ptr(chain)
+            };
 
             Ok(ParsedPkcs12 {
                 pkey: pkey,
@@ -80,6 +85,7 @@ impl Pkcs12 {
 pub struct ParsedPkcs12 {
     pub pkey: PKey,
     pub cert: X509,
+    // FIXME Make this Option<Stack> in the next breaking release
     pub chain: Stack<X509>,
 }
 
@@ -202,7 +208,6 @@ mod test {
         let pkcs12 = Pkcs12::from_der(der).unwrap();
         let parsed = pkcs12.parse("cassandra").unwrap();
 
-
         assert_eq!(parsed.chain.len(), 0);
         assert_eq!(parsed.chain.into_iter().collect::<Vec<_>>().len(), 0);
     }
diff --git a/openssl/src/stack.rs b/openssl/src/stack.rs
index d9795a51..6ac8264c 100644
--- a/openssl/src/stack.rs
+++ b/openssl/src/stack.rs
@@ -86,6 +86,8 @@ impl<T: Stackable> ForeignType for Stack<T> {
 
     #[inline]
     unsafe fn from_ptr(ptr: *mut T::StackType) -> Stack<T> {
+        assert!(!ptr.is_null(), "Must not instantiate a Stack from a null-ptr - use Stack::new() in \
+                                 that case");
         Stack(ptr)
     }
 
@@ -116,7 +118,7 @@ pub struct IntoIter<T: Stackable> {
 
 impl<T: Stackable> IntoIter<T> {
     fn stack_len(&self) -> c_int {
-        safe_stack_size(self.stack as *mut _) as c_int
+        unsafe { OPENSSL_sk_num(self.stack as *mut _) }
     }
 }
 
@@ -154,15 +156,6 @@ impl<T: Stackable> ExactSizeIterator for IntoIter<T> {}
 
 pub struct StackRef<T: Stackable>(Opaque, PhantomData<T>);
 
-fn safe_stack_size(stack: *mut OPENSSL_STACK) -> usize {
-    let l = unsafe { OPENSSL_sk_num(stack) as isize };
-    if l < 0 {
-        0
-    } else {
-        l as usize
-    }
-}
-
 impl<T: Stackable> ForeignTypeRef for StackRef<T> {
     type CType = T::StackType;
 }
@@ -174,7 +167,7 @@ impl<T: Stackable> StackRef<T> {
 
     /// Returns the number of items in the stack
     pub fn len(&self) -> usize {
-        safe_stack_size(self.as_stack())
+        unsafe { OPENSSL_sk_num(self.as_stack()) as usize }
     }
 
     pub fn iter(&self) -> Iter<T> {