Remove Monero torsion-free requirement and make output keys 32 bytes

Maintains the torsion-free requirement in the one place it's used (key 
images).

In the modern day, the output keys are checked to be points, yet in 
older protocol versions they were allowed to be arbitrary bytes.

Closes https://github.com/serai-dex/serai/issues/23 and 
https://github.com/serai-dex/serai/issues/25.
This commit is contained in:
Luke Parker 2022-08-30 01:02:55 -04:00
parent ee6316b26b
commit d620231530
No known key found for this signature in database
GPG key ID: F9F1386DB1E119B6
5 changed files with 28 additions and 20 deletions
coins/monero/src
processor/src/coin

View file

@ -104,11 +104,15 @@ pub(crate) fn read_point<R: io::Read>(r: &mut R) -> io::Result<EdwardsPoint> {
let bytes = read_bytes(r)?;
CompressedEdwardsY(bytes)
.decompress()
// Ban torsioned points, and points which are either unreduced or -0
.filter(|point| point.is_torsion_free() && (point.compress().to_bytes() == bytes))
// Ban points which are either unreduced or -0
.filter(|point| point.compress().to_bytes() == bytes)
.ok_or_else(|| io::Error::new(io::ErrorKind::Other, "invalid point"))
}
pub(crate) fn read_torsion_free_point<R: io::Read>(r: &mut R) -> io::Result<EdwardsPoint> {
read_point(r).ok().filter(|point| point.is_torsion_free()).ok_or(io::Error::new(io::ErrorKind::Other, "invalid point"))
}
pub(crate) fn read_raw_vec<R: io::Read, T, F: Fn(&mut R) -> io::Result<T>>(
f: F,
len: usize,

View file

@ -2,7 +2,7 @@ use core::cmp::Ordering;
use zeroize::Zeroize;
use curve25519_dalek::edwards::EdwardsPoint;
use curve25519_dalek::edwards::{EdwardsPoint, CompressedEdwardsY};
use crate::{
Protocol, hash,
@ -46,7 +46,7 @@ impl Input {
2 => Input::ToKey {
amount: read_varint(r)?,
key_offsets: read_vec(read_varint, r)?,
key_image: read_point(r)?,
key_image: read_torsion_free_point(r)?,
},
_ => Err(std::io::Error::new(
std::io::ErrorKind::Other,
@ -60,7 +60,7 @@ impl Input {
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct Output {
pub amount: u64,
pub key: EdwardsPoint,
pub key: CompressedEdwardsY,
pub view_tag: Option<u8>,
}
@ -72,7 +72,7 @@ impl Output {
pub fn serialize<W: std::io::Write>(&self, w: &mut W) -> std::io::Result<()> {
write_varint(&self.amount, w)?;
w.write_all(&[2 + (if self.view_tag.is_some() { 1 } else { 0 })])?;
write_point(&self.key, w)?;
w.write_all(&self.key.to_bytes())?;
if let Some(view_tag) = self.view_tag {
w.write_all(&[view_tag])?;
}
@ -92,7 +92,7 @@ impl Output {
Ok(Output {
amount,
key: read_point(r)?,
key: CompressedEdwardsY(read_bytes(r)?),
view_tag: if view_tag { Some(read_byte(r)?) } else { None },
})
}

View file

@ -65,8 +65,7 @@ pub struct Metadata {
pub subaddress: (u32, u32),
// Can be an Option, as extra doesn't necessarily have a payment ID, yet all Monero TXs should
// have this
// This will be gibberish if the payment ID wasn't intended for the recipient
// This will be [0xff; 8] if the transaction didn't have a payment ID
// This will be gibberish if the payment ID wasn't intended for the recipient or wasn't included
// 0xff was chosen as it'd be distinct from [0; 8], enabling atomically incrementing IDs (though
// they should be randomly generated)
pub payment_id: [u8; 8],
@ -211,14 +210,19 @@ impl Scanner {
let mut res = vec![];
for (o, output) in tx.prefix.outputs.iter().enumerate() {
// https://github.com/serai-dex/serai/issues/102
let output_key_compressed = output.key.compress();
// https://github.com/serai-dex/serai/issues/106
if let Some(burning_bug) = self.burning_bug.as_ref() {
if burning_bug.contains(&output_key_compressed) {
if burning_bug.contains(&output.key) {
continue;
}
}
let output_key = output.key.decompress();
if output_key.is_none() {
continue;
}
let output_key = output_key.unwrap();
for key in &keys {
let (view_tag, shared_key, payment_id_xor) = shared_key(
if self.burning_bug.is_none() { Some(uniqueness(&tx.prefix.inputs)) } else { None },
@ -231,7 +235,7 @@ impl Scanner {
if let Some(PaymentId::Encrypted(id)) = payment_id.map(|id| id ^ payment_id_xor) {
id
} else {
[0xff; 8]
payment_id_xor
};
if let Some(actual_view_tag) = output.view_tag {
@ -243,15 +247,15 @@ impl Scanner {
// P - shared == spend
let subaddress = self
.subaddresses
.get(&(output.key - (&shared_key * &ED25519_BASEPOINT_TABLE)).compress());
.get(&(output_key - (&shared_key * &ED25519_BASEPOINT_TABLE)).compress());
if subaddress.is_none() {
continue;
}
// If it has torsion, it'll substract the non-torsioned shared key to a torsioned key
// We will not have a torsioned key in our HashMap of keys, so we wouldn't identify it as
// ours
// If we did, it'd enable bypassing the included burning bug protection however
debug_assert!(output.key.is_torsion_free());
// If we did though, it'd enable bypassing the included burning bug protection
debug_assert!(output_key.is_torsion_free());
let key_offset = shared_key + self.pair.subaddress(*subaddress.unwrap());
// Since we've found an output to us, get its amount
@ -282,13 +286,13 @@ impl Scanner {
res.push(ReceivedOutput {
absolute: AbsoluteId { tx: tx.hash(), o: o.try_into().unwrap() },
data: OutputData { key: output.key, key_offset, commitment },
data: OutputData { key: output_key, key_offset, commitment },
metadata: Metadata { subaddress: (0, 0), payment_id },
});
if let Some(burning_bug) = self.burning_bug.as_mut() {
burning_bug.insert(output_key_compressed);
burning_bug.insert(output.key);
}
}
// Break to prevent public keys from being included multiple times, triggering multiple

View file

@ -305,7 +305,7 @@ impl SignableTransaction {
for output in &outputs {
tx_outputs.push(Output {
amount: 0,
key: output.dest,
key: output.dest.compress(),
view_tag: Some(output.view_tag).filter(|_| matches!(self.protocol, Protocol::v16)),
});
ecdh_info.push(output.amount);

View file

@ -199,7 +199,7 @@ impl Coin for Monero {
Ok((
tx.hash().to_vec(),
tx.prefix.outputs.iter().map(|output| output.key.compress().to_bytes()).collect(),
tx.prefix.outputs.iter().map(|output| output.key.to_bytes()).collect(),
))
}