From b54548b13a44396c7eb35d421b7275a9c4623a68 Mon Sep 17 00:00:00 2001 From: Luke Parker <lukeparker5132@gmail.com> Date: Sun, 9 Jul 2023 00:44:23 -0400 Subject: [PATCH] Only deserialize RctSignatures where's there at least one input This is only enforced by the Monero protocol due to a single check the mixRing isn't empty in get_pre_mlsag_hash. The value in ensuring there's a least one input is to ensure the safety of our rct_type functions, which determines the RctType based off structural analysis (specifically, input data if MlsagBorromean). rct_type was technically safe without this. A 0-input transaction would be mis-classified as RctFull/MlsagAggregate, which would then make the RctSignatures invalid for being RctFull (requiring exactly one input) yet not having inputs, meaning an invalid RctSignatures would be mis-classified yet still invalid. This just removes the risk of mis-classification in the first place, tightening the library's safety. --- coins/monero/src/ringct/mod.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/coins/monero/src/ringct/mod.rs b/coins/monero/src/ringct/mod.rs index eeadfa07..6288417d 100644 --- a/coins/monero/src/ringct/mod.rs +++ b/coins/monero/src/ringct/mod.rs @@ -255,6 +255,17 @@ impl RctPrunable { outputs: usize, r: &mut R, ) -> io::Result<RctPrunable> { + // While we generally don't bother with misc consensus checks, this affects the safety of + // the below defined rct_type function + // The exact line preventing zero-input transactions is: + // https://github.com/monero-project/monero/blob/00fd416a99686f0956361d1cd0337fe56e58d4a7/ + // 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() { + Err(io::Error::new(io::ErrorKind::Other, "transaction had no inputs"))?; + } + Ok(match rct_type { RctType::Null => RctPrunable::Null, RctType::MlsagAggregate | RctType::MlsagIndividual => RctPrunable::MlsagBorromean {