ffi-macros: Protect against use-after-free.

  - When we transfer ownership from C to Rust, we move the wrapped
    object out of the wrapper, and poison the wrapper.

  - This prevents reuse of the wrapper object.  When a stale reference
    is given to us, we check the tag encoding the type information.

  - If the tag field is poisoned, we can produce a more helpful error
    message.  This is not exact, of course.  As soon as the memory is
    reused, our tag is overwritten.

8 files changed, 53 insertions, 4 deletions

diff --git a/ffi-macros/src/lib.rs b/ffi-macros/src/lib.rs
index 0d659687..7bedb0ce 100644
--- a/ffi-macros/src/lib.rs
+++ b/ffi-macros/src/lib.rs
@@ -309,7 +309,13 @@ fn derive_conversion_functions(st: &mut syn::ItemStruct, _: &str, _: &str,
         impl #wrapper {
             fn assert_tag(&self) {
                 if self.1 != #magic_value {
-                    panic!("FFI contract violation: Wrong parameter type");
+                    if self.1 == 0x5050505050505050 {
+                        panic!(
+                            "FFI contract violation: Use-after-free detected");
+                    } else {
+                        panic!(
+                            "FFI contract violation: Wrong parameter type");
+                    }
                 }
             }
         }
@@ -320,11 +326,21 @@ fn derive_conversion_functions(st: &mut syn::ItemStruct, _: &str, _: &str,
                 if self.is_null() {
                     panic!("FFI contract violation: Parameter is NULL");
                 }
-                let wrapper = unsafe {
+                let mut wrapper = unsafe {
                     Box::from_raw(self)
                 };
                 wrapper.assert_tag();
-                wrapper.0
+                let obj = wrapper.0;
+
+                // Poison the wrapper.
+                unsafe {
+                    // Overwrite with P.
+                    memsec::memset(self as *mut u8,
+                                   0x50,
+                                   ::std::mem::size_of::<#wrapper>())
+                };
+
+                obj
             }
         }
 
diff --git a/ffi/Cargo.toml b/ffi/Cargo.toml
index 80059a09..2f62738d 100644
--- a/ffi/Cargo.toml
+++ b/ffi/Cargo.toml
@@ -29,6 +29,7 @@ sequoia-net = { path = "../net" }
 failure = "0.1.2"
 lazy_static = "1.0.0"
 libc = "0.2.33"
+memsec = "0.5.4"
 native-tls = "0.2.0"
 time = "0.1.40"
 
diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs
index 9b095cd3..8c88b836 100644
--- a/ffi/src/lib.rs
+++ b/ffi/src/lib.rs
@@ -112,6 +112,7 @@ extern crate failure;
 extern crate lazy_static;
 extern crate libc;
 extern crate native_tls;
+extern crate memsec;
 
 extern crate sequoia_ffi_macros;
 use sequoia_ffi_macros::{
diff --git a/openpgp-ffi/Cargo.toml b/openpgp-ffi/Cargo.toml
index 1bb6aab1..a719871b 100644
--- a/openpgp-ffi/Cargo.toml
+++ b/openpgp-ffi/Cargo.toml
@@ -26,6 +26,7 @@ sequoia-openpgp = { path = "../openpgp" }
 failure = "0.1.2"
 lazy_static = "1.0.0"
 libc = "0.2.33"
+memsec = "0.5.4"
 time = "0.1.40"
 
 [dev-dependencies]
diff --git a/openpgp-ffi/examples/.gitignore b/openpgp-ffi/examples/.gitignore
index c1833f90..3370781f 100644
--- a/openpgp-ffi/examples/.gitignore
+++ b/openpgp-ffi/examples/.gitignore
@@ -4,3 +4,4 @@ example
 parser
 reader
 type-safety-demo
+use-after-free-demo
diff --git a/openpgp-ffi/examples/Makefile b/openpgp-ffi/examples/Makefile
index d935ea52..3b659e50 100644
--- a/openpgp-ffi/examples/Makefile
+++ b/openpgp-ffi/examples/Makefile
@@ -5,7 +5,7 @@ CARGO_TARGET_DIR	?= $(shell pwd)/../../target
 # We currently only support absolute paths.
 CARGO_TARGET_DIR	:= $(abspath $(CARGO_TARGET_DIR))
 
-TARGETS = example reader parser encrypt-for armor type-safety-demo
+TARGETS = example reader parser encrypt-for armor type-safety-demo use-after-free-demo
 CFLAGS  = -I../include -O0 -g -Wall -Werror
 LDFLAGS = -L$(CARGO_TARGET_DIR)/debug -lsequoia_openpgp_ffi
 
diff --git a/openpgp-ffi/examples/use-after-free-demo.c b/openpgp-ffi/examples/use-after-free-demo.c
new file mode 100644
index 00000000..e64a3c1f
--- /dev/null
+++ b/openpgp-ffi/examples/use-after-free-demo.c
@@ -0,0 +1,28 @@
+#define _GNU_SOURCE
+#include <errno.h>
+#include <error.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sequoia/openpgp.h>
+
+int
+main (int argc, char **argv)
+{
+  // Let's make a KeyID!
+  pgp_keyid_t keyid = pgp_keyid_from_hex ("BBBBBBBBBBBBBBBB");
+  printf ("%s", pgp_keyid_to_string (keyid));
+
+  // Always clean up after you played.
+  pgp_keyid_free (keyid);
+
+  // Let's violate The Rules and use the stale reference!
+  printf ("%s", pgp_keyid_to_string (keyid));
+
+  return 0;
+}
diff --git a/openpgp-ffi/src/lib.rs b/openpgp-ffi/src/lib.rs
index 3e8be671..8b406208 100644
--- a/openpgp-ffi/src/lib.rs
+++ b/openpgp-ffi/src/lib.rs
@@ -314,6 +314,7 @@ extern crate failure;
 #[macro_use]
 extern crate lazy_static;
 extern crate libc;
+extern crate memsec;
 
 extern crate sequoia_ffi_macros;
 use sequoia_ffi_macros::{