Skip to content

Commit

Permalink
rangeproof::clone memory corruption and remove rustc
Browse files Browse the repository at this point in the history
  • Loading branch information
vekamo committed Apr 27, 2024
1 parent aeaa9ee commit efc5684
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 154 deletions.
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]

name = "grin_secp256k1zkp"
version = "0.7.12"
version = "0.7.13"
authors = [ "Grin Developers <[email protected]>",
"Dawid Ciężarkiewicz <[email protected]>",
"Andrew Poelstra <[email protected]>"]
Expand Down Expand Up @@ -33,7 +33,6 @@ arrayvec = "0.3"
clippy = {version = "0.0", optional = true}
rand = "0.5"
libc = "0.2"
rustc-serialize = "0.3"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
zeroize = { version = "1.1", features = ["zeroize_derive"] }
Expand Down
99 changes: 0 additions & 99 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use std::marker;
use arrayvec::ArrayVec;
use rand::Rng;
use crate::serialize::{Decoder, Decodable, Encoder, Encodable};
use serde::{Serialize, Deserialize, Serializer, Deserializer};

use super::{Secp256k1, ContextFlag};
Expand Down Expand Up @@ -285,34 +284,6 @@ impl PublicKey {
}
}

impl Decodable for PublicKey {
fn decode<D: Decoder>(d: &mut D) -> Result<PublicKey, D::Error> {
d.read_seq(|d, len| {
if len == constants::UNCOMPRESSED_PUBLIC_KEY_SIZE {
unsafe {
use std::mem;
let mut ret: [u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE] = mem::MaybeUninit::uninit().assume_init();
for i in 0..len {
ret[i] = d.read_seq_elt(i, |d| Decodable::decode(d))?;
}
PublicKey::from_slice(&ret).map_err(|_| d.error("invalid public key"))
}
} else if len == constants::COMPRESSED_PUBLIC_KEY_SIZE {
unsafe {
use std::mem;
let mut ret: [u8; constants::COMPRESSED_PUBLIC_KEY_SIZE] = mem::MaybeUninit::uninit().assume_init();
for i in 0..len {
ret[i] = d.read_seq_elt(i, |d| Decodable::decode(d))?;
}
PublicKey::from_slice(&ret).map_err(|_| d.error("invalid public key"))
}
} else {
Err(d.error("Invalid length"))
}
})
}
}

/// Creates a new public key from a FFI public key
impl From<ffi::PublicKey> for PublicKey {
#[inline]
Expand All @@ -321,13 +292,6 @@ impl From<ffi::PublicKey> for PublicKey {
}
}


impl Encodable for PublicKey {
fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
self.serialize_vec(true).encode(s)
}
}

impl<'de> Deserialize<'de> for PublicKey {
fn deserialize<D>(d: D) -> Result<PublicKey, D::Error>
where D: Deserializer<'de>
Expand Down Expand Up @@ -521,69 +485,6 @@ mod test {
assert_eq!(pk.add_exp_assign(&s, &sk), Err(IncapableContext));
}

#[test]
fn test_bad_deserialize() {
use std::io::Cursor;
use crate::serialize::{json, Decodable};

let zero31 = "[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]".as_bytes();
let json31 = json::Json::from_reader(&mut Cursor::new(zero31)).unwrap();
let zero32 = "[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]".as_bytes();
let json32 = json::Json::from_reader(&mut Cursor::new(zero32)).unwrap();
let zero65 = "[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]".as_bytes();
let json65 = json::Json::from_reader(&mut Cursor::new(zero65)).unwrap();
let string = "\"my key\"".as_bytes();
let json = json::Json::from_reader(&mut Cursor::new(string)).unwrap();

// Invalid length
let mut decoder = json::Decoder::new(json31.clone());
assert!(<PublicKey as Decodable>::decode(&mut decoder).is_err());
let mut decoder = json::Decoder::new(json31.clone());
assert!(<SecretKey as Decodable>::decode(&mut decoder).is_err());
let mut decoder = json::Decoder::new(json32.clone());
assert!(<PublicKey as Decodable>::decode(&mut decoder).is_err());
let mut decoder = json::Decoder::new(json32.clone());
assert!(<SecretKey as Decodable>::decode(&mut decoder).is_ok());
let mut decoder = json::Decoder::new(json65.clone());
assert!(<PublicKey as Decodable>::decode(&mut decoder).is_err());
let mut decoder = json::Decoder::new(json65.clone());
assert!(<SecretKey as Decodable>::decode(&mut decoder).is_err());

// Syntax error
let mut decoder = json::Decoder::new(json.clone());
assert!(<PublicKey as Decodable>::decode(&mut decoder).is_err());
let mut decoder = json::Decoder::new(json.clone());
assert!(<SecretKey as Decodable>::decode(&mut decoder).is_err());
}

#[test]
fn test_serialize() {
use std::io::Cursor;
use crate::serialize::{json, Decodable, Encodable};

macro_rules! round_trip (
($var:ident) => ({
let start = $var;
let mut encoded = String::new();
{
let mut encoder = json::Encoder::new(&mut encoded);
start.encode(&mut encoder).unwrap();
}
let json = json::Json::from_reader(&mut Cursor::new(encoded.as_bytes())).unwrap();
let mut decoder = json::Decoder::new(json);
let decoded = Decodable::decode(&mut decoder);
assert_eq!(Ok(Some(start)), decoded);
})
);

let s = Secp256k1::new();
for _ in 0..500 {
let (sk, pk) = s.generate_keypair(&mut thread_rng()).unwrap();
round_trip!(sk);
round_trip!(pk);
}
}

#[test]
fn test_bad_serde_deserialize() {
use serde::Deserialize;
Expand Down
17 changes: 13 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#[cfg(all(test, feature = "unstable"))] extern crate test;

extern crate arrayvec;
extern crate rustc_serialize as serialize;
extern crate serde;
extern crate serde_json as json;

Expand Down Expand Up @@ -130,7 +129,7 @@ impl Signature {
pub fn from_compact(data: &[u8]) -> Result<Signature, Error> {
let mut ret = unsafe { ffi::Signature::blank() };
if data.len() != 64 {
return Err(Error::InvalidSignature)
return Err(Error::InvalidSignature);
}

unsafe {
Expand Down Expand Up @@ -706,14 +705,24 @@ impl Secp256k1 {
#[cfg(test)]
mod tests {
use rand::{Rng, thread_rng};
use crate::serialize::hex::FromHex;
use crate::key::{SecretKey, PublicKey};
use super::constants;
use super::{Secp256k1, Signature, RecoverableSignature, Message, RecoveryId, ContextFlag};
use super::Error::{InvalidMessage, InvalidPublicKey, IncorrectSignature, InvalidSignature,
IncapableContext};

macro_rules! hex (($hex:expr) => ($hex.from_hex().unwrap()));
macro_rules! hex {
($hex:expr) => {{
let bytes = $hex.as_bytes();
let mut vec = Vec::new();
for i in (0..bytes.len()).step_by(2) {
let high = (bytes[i] as char).to_digit(16).unwrap();
let low = (bytes[i + 1] as char).to_digit(16).unwrap();
vec.push(((high << 4) + low) as u8);
}
vec
}};
}

#[test]
fn recursion_test() {
Expand Down
30 changes: 1 addition & 29 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,34 +143,6 @@ macro_rules! impl_array_newtype {
}
}

impl crate::serialize::Decodable for $thing {
fn decode<D: crate::serialize::Decoder>(d: &mut D) -> Result<$thing, D::Error> {
use crate::serialize::Decodable;

d.read_seq(|d, len| {
if len != $len {
Err(d.error("Invalid length"))
} else {
unsafe {
use std::mem;
let mut ret: [$ty; $len] = mem::MaybeUninit::uninit().assume_init();
for i in 0..len {
ret[i] = d.read_seq_elt(i, |d| Decodable::decode(d))?;
}
Ok($thing(ret))
}
}
})
}
}

impl crate::serialize::Encodable for $thing {
fn encode<S: crate::serialize::Encoder>(&self, s: &mut S)
-> Result<(), S::Error> {
self[..].encode(s)
}
}

impl<'de> ::serde::Deserialize<'de> for $thing {
fn deserialize<D>(d: D) -> Result<$thing, D::Error>
where D: ::serde::Deserializer<'de>
Expand Down Expand Up @@ -256,7 +228,7 @@ macro_rules! map_vec {
($thing:expr, $mapfn:expr ) => {
$thing.iter()
.map($mapfn)
.collect::<Vec<_>>();
.collect::<Vec<_>>()
}
}

Expand Down
21 changes: 1 addition & 20 deletions src/pedersen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl Commitment {
}

/// A range proof. Typically much larger in memory that the above (~5k).
#[derive(Copy)]
#[derive(Clone, Copy)]
pub struct RangeProof {
/// The proof itself, at most 5134 bytes long
pub proof: [u8; constants::MAX_PROOF_SIZE],
Expand All @@ -137,25 +137,6 @@ impl PartialEq for RangeProof {
}
}

impl Clone for RangeProof {
#[inline]
fn clone(&self) -> RangeProof {
unsafe {
use std::ptr::copy_nonoverlapping;
let mut ret: [u8; constants::MAX_PROOF_SIZE] = mem::MaybeUninit::uninit().assume_init();
copy_nonoverlapping(
self.proof.as_ptr(),
ret.as_mut_ptr(),
mem::size_of::<RangeProof>(),
);
RangeProof {
proof: ret,
plen: self.plen,
}
}
}
}

impl ser::Serialize for RangeProof {
fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
where
Expand Down

0 comments on commit efc5684

Please sign in to comment.