Move FROST to Read

Fixes https://github.com/serai-dex/serai/issues/33 and 
https://github.com/serai-dex/serai/issues/35. Also fixes a few potential 
panics/DoS AFAICT.

coins/monero/src/frost.rs

@@ -1,4 +1,4 @@
-use std::{convert::TryInto, io::Cursor};
+use std::io::Read;
 
 use thiserror::Error;
 use rand_core::{RngCore, CryptoRng};
@@ -47,17 +47,14 @@ pub(crate) fn write_dleq<R: RngCore + CryptoRng>(
 }
 
 #[allow(non_snake_case)]
-pub(crate) fn read_dleq(
-  serialized: &[u8],
+pub(crate) fn read_dleq<Re: Read>(
+  serialized: &mut Re,
   H: EdwardsPoint,
   l: u16,
   xG: dfg::EdwardsPoint
 ) -> Result<dfg::EdwardsPoint, MultisigError> {
-  if serialized.len() != 96 {
-    Err(MultisigError::InvalidDLEqProof(l))?;
-  }
-
-  let bytes = (&serialized[.. 32]).try_into().unwrap();
+  let mut bytes = [0; 32];
+  serialized.read_exact(&mut bytes).map_err(|_| MultisigError::InvalidDLEqProof(l))?;
   // dfg ensures the point is torsion free
   let xH = Option::<dfg::EdwardsPoint>::from(
     dfg::EdwardsPoint::from_bytes(&bytes)).ok_or(MultisigError::InvalidDLEqProof(l)
@@ -68,7 +65,7 @@ pub(crate) fn read_dleq(
   }
 
   DLEqProof::<dfg::EdwardsPoint>::deserialize(
-    &mut Cursor::new(&serialized[32 ..])
+    serialized
   ).map_err(|_| MultisigError::InvalidDLEqProof(l))?.verify(
     &mut transcript(),
     Generators::new(dfg::EdwardsPoint::generator(), dfg::EdwardsPoint(H)),

coins/monero/src/ringct/clsag/multisig.rs

@@ -1,5 +1,5 @@
 use core::fmt::Debug;
-use std::sync::{Arc, RwLock};
+use std::{io::Read, sync::{Arc, RwLock}};
 
 use rand_core::{RngCore, CryptoRng, SeedableRng};
 use rand_chacha::ChaCha12Rng;
@@ -104,7 +104,7 @@ impl ClsagMultisig {
     )
   }
 
-  pub fn serialized_len() -> usize {
+  pub const fn serialized_len() -> usize {
     32 + (2 * 32)
   }
 
@@ -136,17 +136,12 @@ impl Algorithm<Ed25519> for ClsagMultisig {
     serialized
   }
 
-  fn process_addendum(
+  fn process_addendum<Re: Read>(
     &mut self,
     view: &FrostView<Ed25519>,
     l: u16,
-    serialized: &[u8]
+    serialized: &mut Re
   ) -> Result<(), FrostError> {
-    if serialized.len() != Self::serialized_len() {
-      // Not an optimal error but...
-      Err(FrostError::InvalidCommitment(l))?;
-    }
-
     if self.image.is_identity().into() {
       self.transcript.domain_separate(b"CLSAG");
       self.input().transcript(&mut self.transcript);
@@ -154,13 +149,14 @@ impl Algorithm<Ed25519> for ClsagMultisig {
     }
 
     self.transcript.append_message(b"participant", &l.to_be_bytes());
-    self.transcript.append_message(b"key_image_share", &serialized[.. 32]);
-    self.image += read_dleq(
+    let image = read_dleq(
       serialized,
       self.H,
       l,
       view.verification_share(l)
     ).map_err(|_| FrostError::InvalidCommitment(l))?.0;
+    self.transcript.append_message(b"key_image_share", image.compress().to_bytes().as_ref());
+    self.image += image;
 
     Ok(())
   }

coins/monero/src/wallet/send/multisig.rs

@@ -1,4 +1,4 @@
-use std::{sync::{Arc, RwLock}, collections::HashMap};
+use std::{io::{Read, Cursor}, sync::{Arc, RwLock}, collections::HashMap};
 
 use rand_core::{RngCore, CryptoRng, SeedableRng};
 use rand_chacha::ChaCha12Rng;
@@ -202,57 +202,57 @@ impl PreprocessMachine for TransactionMachine {
 impl SignMachine<Transaction> for TransactionSignMachine {
   type SignatureMachine = TransactionSignatureMachine;
 
-  fn sign(
+  fn sign<Re: Read>(
     mut self,
-    mut commitments: HashMap<u16, Vec<u8>>,
+    mut commitments: HashMap<u16, Re>,
     msg: &[u8]
   ) -> Result<(TransactionSignatureMachine, Vec<u8>), FrostError> {
     if msg.len() != 0 {
       Err(
         FrostError::InternalError(
-          "message was passed to the TransactionMachine when it generates its own".to_string()
+          "message was passed to the TransactionMachine when it generates its own"
         )
       )?;
     }
 
-    // Add all commitments to the transcript for their entropy
-    // While each CLSAG will do this as they need to for security, they have their own transcripts
-    // cloned from this TX's initial premise's transcript. For our TX transcript to have the CLSAG
-    // data for entropy, it'll have to be added ourselves
-    commitments.insert(self.i, self.our_preprocess);
-    for l in &self.included {
-      self.transcript.append_message(b"participant", &(*l).to_be_bytes());
-      // FROST itself will error if this is None, so let it
-      if let Some(preprocess) = commitments.get(l) {
-        self.transcript.append_message(b"preprocess", preprocess);
-      }
-    }
-
     // FROST commitments and their DLEqs, and the image and its DLEq
-    let clsag_len = (2 * (32 + 32)) + (2 * (32 + 32)) + ClsagMultisig::serialized_len();
-    for (l, commitments) in &commitments {
-      if commitments.len() != (self.clsags.len() * clsag_len) {
-        Err(FrostError::InvalidCommitment(*l))?;
-      }
-    }
+    const CLSAG_LEN: usize = (2 * (32 + 32)) + (2 * (32 + 32)) + ClsagMultisig::serialized_len();
 
     // Convert the unified commitments to a Vec of the individual commitments
-    let mut commitments = (0 .. self.clsags.len()).map(|_| commitments.iter_mut().map(
-      |(l, commitments)| (*l, commitments.drain(.. clsag_len).collect::<Vec<_>>())
-    ).collect::<HashMap<_, _>>()).collect::<Vec<_>>();
-
-    // Calculate the key images
-    // Clsag will parse/calculate/validate this as needed, yet doing so here as well provides
-    // the easiest API overall, as this is where the TX is (which needs the key images in its
-    // message), along with where the outputs are determined (where our change output needs these
-    // to be unique)
     let mut images = vec![EdwardsPoint::identity(); self.clsags.len()];
-    for c in 0 .. self.clsags.len() {
-      for (l, preprocess) in &commitments[c] {
+    let mut commitments = (0 .. self.clsags.len()).map(|c| {
+      let mut buf = [0; CLSAG_LEN];
+      (&self.included).iter().map(|l| {
+        // Add all commitments to the transcript for their entropy
+        // While each CLSAG will do this as they need to for security, they have their own transcripts
+        // cloned from this TX's initial premise's transcript. For our TX transcript to have the CLSAG
+        // data for entropy, it'll have to be added ourselves here
+        self.transcript.append_message(b"participant", &(*l).to_be_bytes());
+        if *l == self.i {
+          buf.copy_from_slice(self.our_preprocess.drain(.. CLSAG_LEN).as_slice());
+        } else {
+          commitments.get_mut(l).ok_or(FrostError::MissingParticipant(*l))?
+            .read_exact(&mut buf).map_err(|_| FrostError::InvalidCommitment(*l))?;
+        }
+        self.transcript.append_message(b"preprocess", &buf);
+
+        // While here, calculate the key image
+        // Clsag will parse/calculate/validate this as needed, yet doing so here as well provides
+        // the easiest API overall, as this is where the TX is (which needs the key images in its
+        // message), along with where the outputs are determined (where our outputs may need
+        // these in order to guarantee uniqueness)
         images[c] += CompressedEdwardsY(
-          preprocess[(clsag_len - 96) .. (clsag_len - 64)].try_into().map_err(|_| FrostError::InvalidCommitment(*l))?
+          buf[(CLSAG_LEN - 96) .. (CLSAG_LEN - 64)].try_into().map_err(|_| FrostError::InvalidCommitment(*l))?
         ).decompress().ok_or(FrostError::InvalidCommitment(*l))?;
-      }
+
+        Ok((*l, Cursor::new(buf)))
+      }).collect::<Result<HashMap<_, _>, _>>()
+    }).collect::<Result<Vec<_>, _>>()?;
+
+    // Remove our preprocess which shouldn't be here. It was just the easiest way to implement the
+    // above
+    for map in commitments.iter_mut() {
+      map.remove(&self.i);
     }
 
     // Create the actual transaction
@@ -345,16 +345,18 @@ impl SignMachine<Transaction> for TransactionSignMachine {
 }
 
 impl SignatureMachine<Transaction> for TransactionSignatureMachine {
-  fn complete(self, mut shares: HashMap<u16, Vec<u8>>) -> Result<Transaction, FrostError> {
+  fn complete<Re: Read>(self, mut shares: HashMap<u16, Re>) -> Result<Transaction, FrostError> {
     let mut tx = self.tx;
     match tx.rct_signatures.prunable {
       RctPrunable::Null => panic!("Signing for RctPrunable::Null"),
       RctPrunable::Clsag { ref mut clsags, ref mut pseudo_outs, .. } => {
         for clsag in self.clsags {
           let (clsag, pseudo_out) = clsag.complete(
-            shares.iter_mut().map(
-              |(l, shares)| (*l, shares.drain(.. 32).collect())
-            ).collect::<HashMap<_, _>>()
+            shares.iter_mut().map(|(l, shares)| {
+              let mut buf = [0; 32];
+              shares.read_exact(&mut buf).map_err(|_| FrostError::InvalidShare(*l))?;
+              Ok((*l, Cursor::new(buf)))
+            }).collect::<Result<HashMap<_, _>, _>>()?
           )?;
           clsags.push(clsag);
           pseudo_outs.push(pseudo_out);