Skip to content

Commit eddfe03

Browse files
apoelstraapoelstra
authored
Merge pull request #151 from elichai/2019-08-Cptr-null
Explicit checks for ZST + null fallbacks
2 parents 5ea300a + d7461e4 commit eddfe03

File tree

8 files changed

+174
-43
lines changed

8 files changed

+174
-43
lines changed

‎src/context.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use core::marker::PhantomData;
2-
use ffi;
2+
use ffi::{self, CPtr};
33
use types::{c_uint, c_void};
44
use Error;
55
use Secp256k1;
@@ -181,7 +181,7 @@ impl<'buf, C: Context + 'buf> Secp256k1<C> {
181181
Ok(Secp256k1 {
182182
ctx: unsafe {
183183
ffi::secp256k1_context_preallocated_create(
184-
buf.as_mut_ptr() as *mut c_void,
184+
buf.as_mut_c_ptr() as *mut c_void,
185185
C::FLAGS)
186186
},
187187
phantom: PhantomData,

‎src/ecdh.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
use core::{ops, ptr};
2020

2121
use key::{SecretKey, PublicKey};
22-
use ffi;
22+
use ffi::{self, CPtr};
2323

2424
/// A tag used for recovering the public key from a compact signature
2525
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -34,8 +34,8 @@ impl SharedSecret {
3434
let res = ffi::secp256k1_ecdh(
3535
ffi::secp256k1_context_no_precomp,
3636
&mut ss,
37-
point.as_ptr(),
38-
scalar.as_ptr(),
37+
point.as_c_ptr(),
38+
scalar.as_c_ptr(),
3939
ffi::secp256k1_ecdh_hash_function_default,
4040
ptr::null_mut(),
4141
);

‎src/ffi.rs‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
//! # FFI bindings
1717
//! Direct bindings to the underlying C library functions. These should
1818
//! not be needed for most users.
19-
use core::{mem, hash, slice};
19+
use core::{mem, hash, slice, ptr};
2020
use types::*;
2121

2222
/// Flag for context to enable no precomputation
@@ -359,6 +359,38 @@ unsafe fn strlen(mut str_ptr: *const c_char) -> usize {
359359
}
360360

361361

362+
/// A trait for producing pointers that will always be valid in C. (assuming NULL pointer is a valid no-op)
363+
/// Rust doesn't promise what pointers does it give to ZST (https://doc.rust-lang.org/nomicon/exotic-sizes.html#zero-sized-types-zsts)
364+
/// In case the type is empty this trait will give a NULL pointer, which should be handled in C.
365+
///
366+
pub(crate) trait CPtr {
367+
type Target;
368+
fn as_c_ptr(&self) -> *const Self::Target;
369+
fn as_mut_c_ptr(&mut self) -> *mut Self::Target;
370+
}
371+
372+
impl<T> CPtr for [T] {
373+
type Target = T;
374+
fn as_c_ptr(&self) -> *const Self::Target {
375+
if self.is_empty() {
376+
ptr::null()
377+
} else {
378+
self.as_ptr()
379+
}
380+
}
381+
382+
fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
383+
if self.is_empty() {
384+
ptr::null::<Self::Target>() as *mut _
385+
} else {
386+
self.as_mut_ptr()
387+
}
388+
}
389+
}
390+
391+
392+
393+
362394
#[cfg(feature = "fuzztarget")]
363395
mod fuzz_dummy {
364396
extern crate std;

‎src/key.rs‎

Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use super::Error::{self, InvalidPublicKey, InvalidSecretKey};
2424
use Signing;
2525
use Verification;
2626
use constants;
27-
use ffi;
27+
use ffi::{self, CPtr};
2828

2929
/// Secret 256-bit key used as `x` in an ECDSA signature
3030
pub struct SecretKey([u8; constants::SECRET_KEY_SIZE]);
@@ -117,7 +117,7 @@ impl SecretKey {
117117
unsafe {
118118
while ffi::secp256k1_ec_seckey_verify(
119119
ffi::secp256k1_context_no_precomp,
120-
data.as_ptr(),
120+
data.as_c_ptr(),
121121
) == 0
122122
{
123123
data = random_32_bytes(rng);
@@ -135,7 +135,7 @@ impl SecretKey {
135135
unsafe {
136136
if ffi::secp256k1_ec_seckey_verify(
137137
ffi::secp256k1_context_no_precomp,
138-
data.as_ptr(),
138+
data.as_c_ptr(),
139139
) == 0
140140
{
141141
return Err(InvalidSecretKey);
@@ -162,8 +162,8 @@ impl SecretKey {
162162
unsafe {
163163
if ffi::secp256k1_ec_privkey_tweak_add(
164164
ffi::secp256k1_context_no_precomp,
165-
self.as_mut_ptr(),
166-
other.as_ptr(),
165+
self.as_mut_c_ptr(),
166+
other.as_c_ptr(),
167167
) != 1
168168
{
169169
Err(Error::InvalidTweak)
@@ -187,8 +187,8 @@ impl SecretKey {
187187
unsafe {
188188
if ffi::secp256k1_ec_privkey_tweak_mul(
189189
ffi::secp256k1_context_no_precomp,
190-
self.as_mut_ptr(),
191-
other.as_ptr(),
190+
self.as_mut_c_ptr(),
191+
other.as_c_ptr(),
192192
) != 1
193193
{
194194
Err(Error::InvalidTweak)
@@ -223,7 +223,7 @@ impl PublicKey {
223223
unsafe {
224224
// We can assume the return value because it's not possible to construct
225225
// an invalid `SecretKey` without transmute trickery or something
226-
let res = ffi::secp256k1_ec_pubkey_create(secp.ctx, &mut pk, sk.as_ptr());
226+
let res = ffi::secp256k1_ec_pubkey_create(secp.ctx, &mut pk, sk.as_c_ptr());
227227
debug_assert_eq!(res, 1);
228228
}
229229
PublicKey(pk)
@@ -232,12 +232,14 @@ impl PublicKey {
232232
/// Creates a public key directly from a slice
233233
#[inline]
234234
pub fn from_slice(data: &[u8]) -> Result<PublicKey, Error> {
235+
if data.is_empty() {return Err(Error::InvalidPublicKey);}
236+
235237
let mut pk = ffi::PublicKey::new();
236238
unsafe {
237239
if ffi::secp256k1_ec_pubkey_parse(
238240
ffi::secp256k1_context_no_precomp,
239241
&mut pk,
240-
data.as_ptr(),
242+
data.as_c_ptr(),
241243
data.len() as usize,
242244
) == 1
243245
{
@@ -259,9 +261,9 @@ impl PublicKey {
259261
let mut ret_len = constants::PUBLIC_KEY_SIZE as usize;
260262
let err = ffi::secp256k1_ec_pubkey_serialize(
261263
ffi::secp256k1_context_no_precomp,
262-
ret.as_mut_ptr(),
264+
ret.as_mut_c_ptr(),
263265
&mut ret_len,
264-
self.as_ptr(),
266+
self.as_c_ptr(),
265267
ffi::SECP256K1_SER_COMPRESSED,
266268
);
267269
debug_assert_eq!(err, 1);
@@ -278,9 +280,9 @@ impl PublicKey {
278280
let mut ret_len = constants::UNCOMPRESSED_PUBLIC_KEY_SIZE as usize;
279281
let err = ffi::secp256k1_ec_pubkey_serialize(
280282
ffi::secp256k1_context_no_precomp,
281-
ret.as_mut_ptr(),
283+
ret.as_mut_c_ptr(),
282284
&mut ret_len,
283-
self.as_ptr(),
285+
self.as_c_ptr(),
284286
ffi::SECP256K1_SER_UNCOMPRESSED,
285287
);
286288
debug_assert_eq!(err, 1);
@@ -303,7 +305,7 @@ impl PublicKey {
303305
}
304306
unsafe {
305307
if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx, &mut self.0 as *mut _,
306-
other.as_ptr()) == 1 {
308+
other.as_c_ptr()) == 1 {
307309
Ok(())
308310
} else {
309311
Err(Error::InvalidTweak)
@@ -325,7 +327,7 @@ impl PublicKey {
325327
}
326328
unsafe {
327329
if ffi::secp256k1_ec_pubkey_tweak_mul(secp.ctx, &mut self.0 as *mut _,
328-
other.as_ptr()) == 1 {
330+
other.as_c_ptr()) == 1 {
329331
Ok(())
330332
} else {
331333
Err(Error::InvalidTweak)
@@ -339,11 +341,11 @@ impl PublicKey {
339341
pub fn combine(&self, other: &PublicKey) -> Result<PublicKey, Error> {
340342
unsafe {
341343
let mut ret = ffi::PublicKey::new();
342-
let ptrs = [self.as_ptr(), other.as_ptr()];
344+
let ptrs = [self.as_c_ptr(), other.as_c_ptr()];
343345
if ffi::secp256k1_ec_pubkey_combine(
344346
ffi::secp256k1_context_no_precomp,
345347
&mut ret,
346-
ptrs.as_ptr(),
348+
ptrs.as_c_ptr(),
347349
2
348350
) == 1
349351
{
@@ -355,6 +357,18 @@ impl PublicKey {
355357
}
356358
}
357359

360+
impl CPtr for PublicKey {
361+
type Target = ffi::PublicKey;
362+
fn as_c_ptr(&self) -> *const Self::Target {
363+
self.as_ptr()
364+
}
365+
366+
fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
367+
self.as_mut_ptr()
368+
}
369+
}
370+
371+
358372
/// Creates a new public key from a FFI public key
359373
impl From<ffi::PublicKey> for PublicKey {
360374
#[inline]
@@ -562,6 +576,36 @@ mod test {
562576
PublicKey::from_slice(&[0x55; constants::PUBLIC_KEY_SIZE]),
563577
Err(InvalidPublicKey)
564578
);
579+
assert_eq!(
580+
PublicKey::from_slice(&[]),
581+
Err(InvalidPublicKey)
582+
);
583+
}
584+
585+
#[test]
586+
fn test_seckey_from_bad_slice() {
587+
// Bad sizes
588+
assert_eq!(
589+
SecretKey::from_slice(&[0; constants::SECRET_KEY_SIZE - 1]),
590+
Err(InvalidSecretKey)
591+
);
592+
assert_eq!(
593+
SecretKey::from_slice(&[0; constants::SECRET_KEY_SIZE + 1]),
594+
Err(InvalidSecretKey)
595+
);
596+
// Bad parse
597+
assert_eq!(
598+
SecretKey::from_slice(&[0xff; constants::SECRET_KEY_SIZE]),
599+
Err(InvalidSecretKey)
600+
);
601+
assert_eq!(
602+
SecretKey::from_slice(&[0x00; constants::SECRET_KEY_SIZE]),
603+
Err(InvalidSecretKey)
604+
);
605+
assert_eq!(
606+
SecretKey::from_slice(&[]),
607+
Err(InvalidSecretKey)
608+
);
565609
}
566610

567611
#[test]

‎src/lib.rs‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ pub use key::PublicKey;
161161
pub use context::*;
162162
use core::marker::PhantomData;
163163
use core::ops::Deref;
164+
use ffi::CPtr;
164165

165166
/// An ECDSA signature
166167
#[derive(Copy, Clone, PartialEq, Eq)]
@@ -246,13 +247,15 @@ impl Signature {
246247
#[inline]
247248
/// Converts a DER-encoded byte slice to a signature
248249
pub fn from_der(data: &[u8]) -> Result<Signature, Error> {
250+
if data.is_empty() {return Err(Error::InvalidSignature);}
251+
249252
let mut ret = ffi::Signature::new();
250253

251254
unsafe {
252255
if ffi::secp256k1_ecdsa_signature_parse_der(
253256
ffi::secp256k1_context_no_precomp,
254257
&mut ret,
255-
data.as_ptr(),
258+
data.as_c_ptr(),
256259
data.len() as usize,
257260
) == 1
258261
{
@@ -274,7 +277,7 @@ impl Signature {
274277
if ffi::secp256k1_ecdsa_signature_parse_compact(
275278
ffi::secp256k1_context_no_precomp,
276279
&mut ret,
277-
data.as_ptr(),
280+
data.as_c_ptr(),
278281
) == 1
279282
{
280283
Ok(Signature(ret))
@@ -289,12 +292,14 @@ impl Signature {
289292
/// 2016. It should never be used in new applications. This library does not
290293
/// support serializing to this "format"
291294
pub fn from_der_lax(data: &[u8]) -> Result<Signature, Error> {
295+
if data.is_empty() {return Err(Error::InvalidSignature);}
296+
292297
unsafe {
293298
let mut ret = ffi::Signature::new();
294299
if ffi::ecdsa_signature_parse_der_lax(
295300
ffi::secp256k1_context_no_precomp,
296301
&mut ret,
297-
data.as_ptr(),
302+
data.as_c_ptr(),
298303
data.len() as usize,
299304
) == 1
300305
{
@@ -328,8 +333,8 @@ impl Signature {
328333
// was already normalized. We don't care.
329334
ffi::secp256k1_ecdsa_signature_normalize(
330335
ffi::secp256k1_context_no_precomp,
331-
self.as_mut_ptr(),
332-
self.as_ptr(),
336+
self.as_mut_c_ptr(),
337+
self.as_c_ptr(),
333338
);
334339
}
335340
}
@@ -356,7 +361,7 @@ impl Signature {
356361
ffi::secp256k1_context_no_precomp,
357362
ret.get_data_mut_ptr(),
358363
&mut len,
359-
self.as_ptr(),
364+
self.as_c_ptr(),
360365
);
361366
debug_assert!(err == 1);
362367
ret.set_len(len);
@@ -371,15 +376,26 @@ impl Signature {
371376
unsafe {
372377
let err = ffi::secp256k1_ecdsa_signature_serialize_compact(
373378
ffi::secp256k1_context_no_precomp,
374-
ret.as_mut_ptr(),
375-
self.as_ptr(),
379+
ret.as_mut_c_ptr(),
380+
self.as_c_ptr(),
376381
);
377382
debug_assert!(err == 1);
378383
}
379384
ret
380385
}
381386
}
382387

388+
impl CPtr for Signature {
389+
type Target = ffi::Signature;
390+
fn as_c_ptr(&self) -> *const Self::Target {
391+
self.as_ptr()
392+
}
393+
394+
fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
395+
self.as_mut_ptr()
396+
}
397+
}
398+
383399
/// Creates a new signature from a FFI signature
384400
impl From<ffi::Signature> for Signature {
385401
#[inline]
@@ -583,7 +599,7 @@ impl<C: Context> Secp256k1<C> {
583599
let mut seed = [0; 32];
584600
rng.fill_bytes(&mut seed);
585601
unsafe {
586-
let err = ffi::secp256k1_context_randomize(self.ctx, seed.as_ptr());
602+
let err = ffi::secp256k1_context_randomize(self.ctx, seed.as_c_ptr());
587603
// This function cannot fail; it has an error return for future-proofing.
588604
// We do not expose this error since it is impossible to hit, and we have
589605
// precedent for not exposing impossible errors (for example in
@@ -609,8 +625,8 @@ impl<C: Signing> Secp256k1<C> {
609625
unsafe {
610626
// We can assume the return value because it's not possible to construct
611627
// an invalid signature from a valid `Message` and `SecretKey`
612-
assert_eq!(ffi::secp256k1_ecdsa_sign(self.ctx, &mut ret, msg.as_ptr(),
613-
sk.as_ptr(), ffi::secp256k1_nonce_function_rfc6979,
628+
assert_eq!(ffi::secp256k1_ecdsa_sign(self.ctx, &mut ret, msg.as_c_ptr(),
629+
sk.as_c_ptr(), ffi::secp256k1_nonce_function_rfc6979,
614630
ptr::null()), 1);
615631
}
616632

@@ -640,7 +656,7 @@ impl<C: Verification> Secp256k1<C> {
640656
#[inline]
641657
pub fn verify(&self, msg: &Message, sig: &Signature, pk: &key::PublicKey) -> Result<(), Error> {
642658
unsafe {
643-
if ffi::secp256k1_ecdsa_verify(self.ctx, sig.as_ptr(), msg.as_ptr(), pk.as_ptr()) == 0 {
659+
if ffi::secp256k1_ecdsa_verify(self.ctx, sig.as_c_ptr(), msg.as_c_ptr(), pk.as_c_ptr()) == 0 {
644660
Err(Error::IncorrectSignature)
645661
} else {
646662
Ok(())

0 commit comments

Comments
 (0)