From ea08dd76a520bc385e451be06e5f55d7efd644c8 Mon Sep 17 00:00:00 2001
From: Robert Habermeier <rphmeier@gmail.com>
Date: Sat, 28 May 2016 21:48:42 +0200
Subject: [PATCH] remove all possible unsafe code in crypto (#1168)

* use #[repr(C)] for all hash types

* use a zeroed buffer in crypto::ec::sign

* eliminate most usages of unsafe in crypto::ecdh::agree

* eliminate all possible unsafety in crypto module
---
 util/src/crypto.rs | 54 +++++++++++++++++++++++++++-------------------
 util/src/hash.rs   |  1 +
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/util/src/crypto.rs b/util/src/crypto.rs
index 3eff70717..b92bb2bae 100644
--- a/util/src/crypto.rs
+++ b/util/src/crypto.rs
@@ -38,13 +38,10 @@ lazy_static! {
 impl Signature {
 	/// Create a new signature from the R, S and V componenets.
 	pub fn from_rsv(r: &H256, s: &H256, v: u8) -> Signature {
-		use std::ptr;
 		let mut ret: Signature = Signature::new();
-		unsafe {
-			let retslice: &mut [u8] = &mut ret;
-			ptr::copy(r.as_ptr(), retslice.as_mut_ptr(), 32);
-			ptr::copy(s.as_ptr(), retslice.as_mut_ptr().offset(32), 32);
-		}
+		(&mut ret[0..32]).copy_from_slice(r);
+		(&mut ret[32..64]).copy_from_slice(s);
+
 		ret[64] = v;
 		ret
 	}
@@ -145,7 +142,10 @@ impl KeyPair {
 		let (sec, publ) = try!(context.generate_keypair(&mut rng));
 		let serialized = publ.serialize_vec(context, false);
 		let p: Public = Public::from_slice(&serialized[1..65]);
-		let s: Secret = unsafe { ::std::mem::transmute(sec) };
+
+		let mut s = Secret::new();
+		s.copy_from_slice(&sec[0..32]);
+
 		Ok(KeyPair {
 			secret: s,
 			public: p,
@@ -193,12 +193,14 @@ pub mod ec {
 	/// Returns siganture of message hash.
 	pub fn sign(secret: &Secret, message: &H256) -> Result<Signature, CryptoError> {
 		// TODO: allow creation of only low-s signatures.
-		use secp256k1::*;
+		use secp256k1::{Message, key};
+
 		let context = &crypto::SECP256K1;
+		// no way to create from raw byte array.
 		let sec: &key::SecretKey = unsafe { ::std::mem::transmute(secret) };
 		let s = try!(context.sign_recoverable(&try!(Message::from_slice(&message)), sec));
 		let (rec_id, data) = s.serialize_compact(context);
-		let mut signature: crypto::Signature = unsafe { ::std::mem::uninitialized() };
+		let mut signature = crypto::Signature::new();
 		signature.clone_from_slice(&data);
 		signature[64] = rec_id.to_i32() as u8;
 
@@ -217,10 +219,12 @@ pub mod ec {
 		let rsig = try!(RecoverableSignature::from_compact(context, &signature[0..64], try!(RecoveryId::from_i32(signature[64] as i32))));
 		let sig = rsig.to_standard(context);
 
-		let mut pdata: [u8; 65] = [4u8; 65];
-		let ptr = pdata[1..].as_mut_ptr();
-		let src = public.as_ptr();
-		unsafe { ::std::ptr::copy_nonoverlapping(src, ptr, 64) };
+		let pdata: [u8; 65] = {
+			let mut temp = [4u8; 65];
+			(&mut temp[1..65]).copy_from_slice(public);
+			temp
+		};
+
 		let publ = try!(key::PublicKey::from_slice(context, &pdata));
 		match context.verify(&try!(Message::from_slice(&message)), &sig, &publ) {
 			Ok(_) => Ok(true),
@@ -252,21 +256,27 @@ pub mod ec {
 /// ECDH functions
 #[cfg_attr(feature="dev", allow(similar_names))]
 pub mod ecdh {
-	use crypto::*;
-	use crypto::{self};
+	use hash::FixedHash;
+	use crypto::{self, Secret, Public, CryptoError};
 
 	/// Agree on a shared secret
-	pub fn agree(secret: &Secret, public: &Public, ) -> Result<Secret, CryptoError> {
-		use secp256k1::*;
+	pub fn agree(secret: &Secret, public: &Public) -> Result<Secret, CryptoError> {
+		use secp256k1::{ecdh, key};
+
 		let context = &crypto::SECP256K1;
-		let mut pdata: [u8; 65] = [4u8; 65];
-		let ptr = pdata[1..].as_mut_ptr();
-		let src = public.as_ptr();
-		unsafe { ::std::ptr::copy_nonoverlapping(src, ptr, 64) };
+		let pdata = {
+			let mut temp = [4u8; 65];
+			(&mut temp[1..65]).copy_from_slice(&public[0..64]);
+			temp
+		};
+
 		let publ = try!(key::PublicKey::from_slice(context, &pdata));
+		// no way to create SecretKey from raw byte array.
 		let sec: &key::SecretKey = unsafe { ::std::mem::transmute(secret) };
 		let shared = ecdh::SharedSecret::new_raw(context, &publ, &sec);
-		let s: Secret = unsafe { ::std::mem::transmute(shared) };
+
+		let mut s = crypto::Secret::new();
+		s.copy_from_slice(&shared[0..32]);
 		Ok(s)
 	}
 }
diff --git a/util/src/hash.rs b/util/src/hash.rs
index e1b82b14c..ba96f812c 100644
--- a/util/src/hash.rs
+++ b/util/src/hash.rs
@@ -75,6 +75,7 @@ pub fn clean_0x(s: &str) -> &str {
 macro_rules! impl_hash {
 	($from: ident, $size: expr) => {
 		#[derive(Eq)]
+		#[repr(C)]
 		/// Unformatted binary data of fixed length.
 		pub struct $from (pub [u8; $size]);