diff --git a/coins/monero/rpc/src/lib.rs b/coins/monero/rpc/src/lib.rs index a8dde92b2..2bdcc0bb1 100644 --- a/coins/monero/rpc/src/lib.rs +++ b/coins/monero/rpc/src/lib.rs @@ -220,23 +220,6 @@ fn rpc_point(point: &str) -> Result { .ok_or_else(|| RpcError::InvalidNode(format!("invalid point: {point}"))) } -// Read an EPEE VarInt, distinct from the VarInts used throughout the rest of the protocol -fn read_epee_vi(reader: &mut R) -> io::Result { - let vi_start = read_byte(reader)?; - let len = match vi_start & 0b11 { - 0 => 1, - 1 => 2, - 2 => 4, - 3 => 8, - _ => unreachable!(), - }; - let mut vi = u64::from(vi_start >> 2); - for i in 1 .. len { - vi |= u64::from(read_byte(reader)?) << (((i - 1) * 8) + 6); - } - Ok(vi) -} - /// An RPC connection to a Monero daemon. /// /// This is abstract such that users can use an HTTP library (which being their choice), a @@ -511,26 +494,29 @@ pub trait Rpc: Sync + Clone + Debug { /// Get the output indexes of the specified transaction. async fn get_o_indexes(&self, hash: [u8; 32]) -> Result, RpcError> { - /* - TODO: Use these when a suitable epee serde lib exists - - #[derive(Serialize, Debug)] - struct Request { - txid: [u8; 32], - } - - #[derive(Deserialize, Debug)] - struct OIndexes { - o_indexes: Vec, - } - */ - // Given the immaturity of Rust epee libraries, this is a homegrown one which is only validated // to work against this specific function // Header for EPEE, an 8-byte magic and a version const EPEE_HEADER: &[u8] = b"\x01\x11\x01\x01\x01\x01\x02\x01\x01"; + // Read an EPEE VarInt, distinct from the VarInts used throughout the rest of the protocol + fn read_epee_vi(reader: &mut R) -> io::Result { + let vi_start = read_byte(reader)?; + let len = match vi_start & 0b11 { + 0 => 1, + 1 => 2, + 2 => 4, + 3 => 8, + _ => unreachable!(), + }; + let mut vi = u64::from(vi_start >> 2); + for i in 1 .. len { + vi |= u64::from(read_byte(reader)?) << (((i - 1) * 8) + 6); + } + Ok(vi) + } + let mut request = EPEE_HEADER.to_vec(); // Number of fields (shifted over 2 bits as the 2 LSBs are reserved for metadata) request.push(1 << 2); @@ -550,29 +536,58 @@ pub trait Rpc: Sync + Clone + Debug { (|| { let mut res = None; - let mut is_okay = false; + let mut has_status = false; if read_bytes::<_, { EPEE_HEADER.len() }>(&mut indexes)? != EPEE_HEADER { Err(io::Error::other("invalid header"))?; } let read_object = |reader: &mut &[u8]| -> io::Result> { + // Read the amount of fields let fields = read_byte(reader)? >> 2; for _ in 0 .. fields { + // Read the length of the field's name let name_len = read_byte(reader)?; + // Read the name of the field let name = read_raw_vec(read_byte, name_len.into(), reader)?; let type_with_array_flag = read_byte(reader)?; + // The type of this field, without the potentially set array flag let kind = type_with_array_flag & (!0x80); - - let iters = if type_with_array_flag != kind { read_epee_vi(reader)? } else { 1 }; - - if (&name == b"o_indexes") && (kind != 5) { - Err(io::Error::other("o_indexes weren't u64s"))?; + let has_array_flag = type_with_array_flag != kind; + + // Read this many instances of the field + let iters = if has_array_flag { read_epee_vi(reader)? } else { 1 }; + + // Check the field type + { + #[allow(clippy::match_same_arms)] + let (expected_type, expected_array_flag) = match name.as_slice() { + b"o_indexes" => (5, true), + b"status" => (10, false), + b"untrusted" => (11, false), + b"credits" => (5, false), + b"top_hash" => (10, false), + // On-purposely prints name as a byte vector to prevent printing arbitrary strings + // This is a self-describing format so we don't have to error here, yet we don't + // claim this to be a complete deserialization function + // To ensure it works for this specific use case, it's best to ensure it's limited + // to this specific use case (ensuring we have less variables to deal with) + _ => Err(io::Error::other(format!("unrecognized field in get_o_indexes: {name:?}")))?, + }; + if (expected_type != kind) || (expected_array_flag != has_array_flag) { + let fmt_array_bool = |array_bool| if array_bool { "array" } else { "not array" }; + Err(io::Error::other(format!( + "field {name:?} was {kind} ({}), expected {expected_type} ({})", + fmt_array_bool(has_array_flag), + fmt_array_bool(expected_array_flag) + )))?; + } } - let f = match kind { + let read_field_as_bytes = match kind { + /* // i64 1 => |reader: &mut &[u8]| read_raw_vec(read_byte, 8, reader), // i32 @@ -581,8 +596,10 @@ pub trait Rpc: Sync + Clone + Debug { 3 => |reader: &mut &[u8]| read_raw_vec(read_byte, 2, reader), // i8 4 => |reader: &mut &[u8]| read_raw_vec(read_byte, 1, reader), + */ // u64 5 => |reader: &mut &[u8]| read_raw_vec(read_byte, 8, reader), + /* // u32 6 => |reader: &mut &[u8]| read_raw_vec(read_byte, 4, reader), // u16 @@ -591,6 +608,7 @@ pub trait Rpc: Sync + Clone + Debug { 8 => |reader: &mut &[u8]| read_raw_vec(read_byte, 1, reader), // double 9 => |reader: &mut &[u8]| read_raw_vec(read_byte, 8, reader), + */ // string, or any collection of bytes 10 => |reader: &mut &[u8]| { let len = read_epee_vi(reader)?; @@ -602,55 +620,47 @@ pub trait Rpc: Sync + Clone + Debug { }, // bool 11 => |reader: &mut &[u8]| read_raw_vec(read_byte, 1, reader), + /* // object, errors here as it shouldn't be used on this call 12 => { |_: &mut &[u8]| Err(io::Error::other("node used object in reply to get_o_indexes")) } // array, so far unused 13 => |_: &mut &[u8]| Err(io::Error::other("node used the unused array type")), + */ _ => |_: &mut &[u8]| Err(io::Error::other("node used an invalid type")), }; let mut bytes_res = vec![]; for _ in 0 .. iters { - bytes_res.push(f(reader)?); + bytes_res.push(read_field_as_bytes(reader)?); } let mut actual_res = Vec::with_capacity(bytes_res.len()); match name.as_slice() { b"o_indexes" => { for o_index in bytes_res { - actual_res.push(u64::from_le_bytes( - o_index - .try_into() - .map_err(|_| io::Error::other("node didn't provide 8 bytes for a u64"))?, - )); + actual_res.push(read_u64(&mut o_index.as_slice())?); } res = Some(actual_res); } b"status" => { if bytes_res .first() - .ok_or_else(|| io::Error::other("status wasn't a string"))? + .ok_or_else(|| io::Error::other("status was a 0-length array"))? .as_slice() != b"OK" { - // TODO: Better handle non-OK responses Err(io::Error::other("response wasn't OK"))?; } - is_okay = true; + has_status = true; } - _ => continue, - } - - if is_okay && res.is_some() { - break; + b"untrusted" | b"credits" | b"top_hash" => continue, + _ => Err(io::Error::other("unrecognized field in get_o_indexes"))?, } } - // Didn't return a response with a status - // (if the status wasn't okay, we would've already errored) - if !is_okay { + if !has_status { Err(io::Error::other("response didn't contain a status"))?; } @@ -661,7 +671,7 @@ pub trait Rpc: Sync + Clone + Debug { read_object(&mut indexes) })() - .map_err(|_| RpcError::InvalidNode("invalid binary response".to_string())) + .map_err(|e| RpcError::InvalidNode(format!("invalid binary response: {e:?}"))) } /// Get the output distribution.