Skip to content

Commit

Permalink
Prevent panic when creating a Schnorr from slice (#1056)
Browse files Browse the repository at this point in the history
Introduced new safe builders, same as bign256 and sm2:
- `split_at` can panic if it receives a slice shorter than `Self::BYTES / 2`
- `split_at` is now use in the new `from_bytes` that accepts only byte arrays
  or the right size
- `from_slice` uses a safe conversion that does not panic when the slice is too short
- `try_from` slice uses `from_slice` internally, so it does not panic when a short slice
  is passed
- introduce safe type conversion from SignatureBytes
  • Loading branch information
germanop committed Jun 28, 2024
1 parent cb0d6ba commit bb1ee04
Showing 1 changed file with 48 additions and 12 deletions.
60 changes: 48 additions & 12 deletions k256/src/schnorr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,31 @@ impl Signature {
fn split(&self) -> (&FieldElement, &NonZeroScalar) {
(self.r(), self.s())
}

/// Parse a Secp256k1 signature from a byte array.
pub fn from_bytes(bytes: &SignatureBytes) -> Result<Self> {
let (r_bytes, s_bytes) = bytes.split_at(Self::BYTE_SIZE / 2);

let r: FieldElement =
Option::from(FieldElement::from_bytes(FieldBytes::from_slice(r_bytes)))
.ok_or_else(Error::new)?;

// one of the rules for valid signatures: !is_infinite(R);
if r.is_zero().into() {
return Err(Error::new());
}

let s = NonZeroScalar::try_from(s_bytes).map_err(|_| Error::new())?;

Ok(Self { r, s })
}

/// Parse a Secp256k1 signature from a byte slice.
pub fn from_slice(bytes: &[u8]) -> Result<Self> {
SignatureBytes::try_from(bytes)
.map_err(|_| Error::new())?
.try_into()
}
}

impl Eq for Signature {}
Expand All @@ -139,24 +164,27 @@ impl PartialEq for Signature {
}
}

impl TryFrom<&[u8]> for Signature {
impl TryFrom<SignatureBytes> for Signature {
type Error = Error;

fn try_from(bytes: &[u8]) -> Result<Signature> {
let (r_bytes, s_bytes) = bytes.split_at(Self::BYTE_SIZE / 2);
fn try_from(signature: SignatureBytes) -> Result<Signature> {
Signature::from_bytes(&signature)
}
}

let r: FieldElement =
Option::from(FieldElement::from_bytes(FieldBytes::from_slice(r_bytes)))
.ok_or_else(Error::new)?;
impl TryFrom<&SignatureBytes> for Signature {
type Error = Error;

// one of the rules for valid signatures: !is_infinite(R);
if r.is_zero().into() {
return Err(Error::new());
}
fn try_from(signature: &SignatureBytes) -> Result<Signature> {
Signature::from_bytes(signature)
}
}

let s = NonZeroScalar::try_from(s_bytes).map_err(|_| Error::new())?;
impl TryFrom<&[u8]> for Signature {
type Error = Error;

Ok(Self { r, s })
fn try_from(bytes: &[u8]) -> Result<Signature> {
Signature::from_slice(bytes)
}
}

Expand Down Expand Up @@ -509,4 +537,12 @@ mod tests {
);
}
}

#[test]
fn try_from() {
// Pass an invalid signature (shorter than Self::BYTES / 2) and make sure
// it does not panic, but return Err
let invalid_signature = [111; 24];
assert_eq!(Signature::try_from(&invalid_signature[..]).is_err(), true);
}
}

0 comments on commit bb1ee04

Please sign in to comment.