diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2019-01-25 13:25:30 +0100 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2019-01-25 14:04:26 +0100 |
commit | 2a162dcaf165e59b72a24825bdc2e1c627979d23 (patch) | |
tree | c37e20eb031a33940baa89b2b823169b6553a511 | |
parent | 118a86b0302b4bb35b1f1bbef50b2a042f239384 (diff) |
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.
-rw-r--r-- | ffi-macros/src/lib.rs | 22 | ||||
-rw-r--r-- | ffi/Cargo.toml | 1 | ||||
-rw-r--r-- | ffi/src/lib.rs | 1 | ||||
-rw-r--r-- | openpgp-ffi/Cargo.toml | 1 | ||||
-rw-r--r-- | openpgp-ffi/examples/.gitignore | 1 | ||||
-rw-r--r-- | openpgp-ffi/examples/Makefile | 2 | ||||
-rw-r--r-- | openpgp-ffi/examples/use-after-free-demo.c | 28 | ||||
-rw-r--r-- | openpgp-ffi/src/lib.rs | 1 |
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::{ |