From 93e85c5ce60f67bb2d89b54699cf187982cc2254 Mon Sep 17 00:00:00 2001 From: Boog900 <54e72d8a-345f-4599-bd90-c6b9bc7d0ec5@aleeas.com> Date: Fri, 5 Jan 2024 05:02:16 +0000 Subject: [PATCH] Monero: use only the first input ring length for RCT deserialization. (#504) * Use only the first input ring length for all RCT input signatures. This is what Monero does: https://github.com/monero-project/monero/blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/src/ringct/rctTypes.h#L422 https://github.com/monero-project/monero/blob/master/src/cryptonote_basic/cryptonote_basic.h#L308-L309 This isn't an issue for current transactions as from hf 12 Monero requires all inputs to have the same number of decoys but for transactions before that Monero would reject RCT txs with differing ring lengths. Monero would deserialize each inputs signature using the ring length of the first so the signatures for inputs other than the first would have a different (wrong) number of elements for that input meaning the signature is invalid. But as we are using the ring length of each input, which arguably is the *correct* way, we would approve of transactions with inputs differing in ring lengths. * Check that there is more than one ring member for MLSAG signatures. https://github.com/monero-project/monero/blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/src/ringct/rctSigs.cpp#L462 --- coins/monero/src/ringct/mlsag.rs | 5 ++++- coins/monero/src/ringct/mod.rs | 33 +++++++++++++++++++++----------- coins/monero/src/transaction.rs | 13 +++++-------- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/coins/monero/src/ringct/mlsag.rs b/coins/monero/src/ringct/mlsag.rs index d3b4080e..e5f00bf7 100644 --- a/coins/monero/src/ringct/mlsag.rs +++ b/coins/monero/src/ringct/mlsag.rs @@ -33,7 +33,10 @@ pub struct RingMatrix { impl RingMatrix { pub fn new(matrix: Vec>) -> Result { - if matrix.is_empty() { + // Monero requires that there is more than one ring member for MLSAG signatures: + // https://github.com/monero-project/monero/blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/ + // src/ringct/rctSigs.cpp#L462 + if matrix.len() < 2 { Err(MlsagError::InvalidRing)?; } for member in &matrix { diff --git a/coins/monero/src/ringct/mod.rs b/coins/monero/src/ringct/mod.rs index 4c5a3a0b..bcd7f0c8 100644 --- a/coins/monero/src/ringct/mod.rs +++ b/coins/monero/src/ringct/mod.rs @@ -257,7 +257,8 @@ impl RctPrunable { pub fn read( rct_type: RctType, - decoys: &[usize], + ring_length: usize, + inputs: usize, outputs: usize, r: &mut R, ) -> io::Result { @@ -268,7 +269,7 @@ impl RctPrunable { // src/ringct/rctSigs.cpp#L609 // And then for RctNull, that's only allowed for miner TXs which require one input of // Input::Gen - if decoys.is_empty() { + if inputs == 0 { Err(io::Error::other("transaction had no inputs"))?; } @@ -276,11 +277,11 @@ impl RctPrunable { RctType::Null => RctPrunable::Null, RctType::MlsagAggregate => RctPrunable::AggregateMlsagBorromean { borromean: read_raw_vec(BorromeanRange::read, outputs, r)?, - mlsag: Mlsag::read(decoys[0], decoys.len() + 1, r)?, + mlsag: Mlsag::read(ring_length, inputs + 1, r)?, }, RctType::MlsagIndividual => RctPrunable::MlsagBorromean { borromean: read_raw_vec(BorromeanRange::read, outputs, r)?, - mlsags: decoys.iter().map(|d| Mlsag::read(*d, 2, r)).collect::>()?, + mlsags: (0 .. inputs).map(|_| Mlsag::read(ring_length, 2, r)).collect::>()?, }, RctType::Bulletproofs | RctType::BulletproofsCompactAmount => { RctPrunable::MlsagBulletproofs { @@ -295,8 +296,10 @@ impl RctPrunable { } Bulletproofs::read(r)? }, - mlsags: decoys.iter().map(|d| Mlsag::read(*d, 2, r)).collect::>()?, - pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?, + mlsags: (0 .. inputs) + .map(|_| Mlsag::read(ring_length, 2, r)) + .collect::>()?, + pseudo_outs: read_raw_vec(read_point, inputs, r)?, } } RctType::Clsag | RctType::BulletproofsPlus => RctPrunable::Clsag { @@ -308,8 +311,8 @@ impl RctPrunable { r, )? }, - clsags: (0 .. decoys.len()).map(|o| Clsag::read(decoys[o], r)).collect::>()?, - pseudo_outs: read_raw_vec(read_point, decoys.len(), r)?, + clsags: (0 .. inputs).map(|_| Clsag::read(ring_length, r)).collect::>()?, + pseudo_outs: read_raw_vec(read_point, inputs, r)?, }, }) } @@ -382,8 +385,16 @@ impl RctSignatures { serialized } - pub fn read(decoys: &[usize], outputs: usize, r: &mut R) -> io::Result { - let base = RctBase::read(decoys.len(), outputs, r)?; - Ok(RctSignatures { base: base.0, prunable: RctPrunable::read(base.1, decoys, outputs, r)? }) + pub fn read( + ring_length: usize, + inputs: usize, + outputs: usize, + r: &mut R, + ) -> io::Result { + let base = RctBase::read(inputs, outputs, r)?; + Ok(RctSignatures { + base: base.0, + prunable: RctPrunable::read(base.1, ring_length, inputs, outputs, r)?, + }) } } diff --git a/coins/monero/src/transaction.rs b/coins/monero/src/transaction.rs index 116e0f6a..20ad4009 100644 --- a/coins/monero/src/transaction.rs +++ b/coins/monero/src/transaction.rs @@ -331,14 +331,11 @@ impl Transaction { } } else if prefix.version == 2 { rct_signatures = RctSignatures::read( - &prefix - .inputs - .iter() - .map(|input| match input { - Input::Gen(_) => 0, - Input::ToKey { key_offsets, .. } => key_offsets.len(), - }) - .collect::>(), + prefix.inputs.first().map_or(0, |input| match input { + Input::Gen(_) => 0, + Input::ToKey { key_offsets, .. } => key_offsets.len(), + }), + prefix.inputs.len(), prefix.outputs.len(), r, )?;