From: Valentine Wallace Date: Fri, 5 Aug 2022 21:50:02 +0000 (-0400) Subject: Fix bug in onion message payload decode X-Git-Tag: v0.0.111~27^2~1 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=351349c845b9927ef8a508c1216efe380c41b138;p=rust-lightning Fix bug in onion message payload decode Previously, we were decoding payload lengths as a VarInt. Per the spec, this is wrong -- it should be decoded as a BigSize. This bug also exists in our payment payload decoding, to be fixed separately. Upcoming reply path tests caught this bug because we hadn't encoded a payload greater than 253 before, so we hadn't hit the problem that VarInts are encoded as little-endian whereas BigSizes are encoded as big-endian. --- diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index d4ba28c8..5afe5781 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -16,7 +16,7 @@ use ln::msgs::DecodeError; use ln::onion_utils; use super::blinded_route::{ForwardTlvs, ReceiveTlvs}; use util::chacha20poly1305rfc::{ChaChaPolyReadAdapter, ChaChaPolyWriteAdapter}; -use util::ser::{FixedLengthReader, LengthRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, Writeable, Writer}; +use util::ser::{BigSize, FixedLengthReader, LengthRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, Writeable, Writer}; use core::cmp; use io::{self, Read}; @@ -161,13 +161,7 @@ impl Writeable for (Payload, [u8; 32]) { // Uses the provided secret to simultaneously decode and decrypt the control TLVs. impl ReadableArgs for Payload { fn read(mut r: &mut R, encrypted_tlvs_ss: SharedSecret) -> Result { - use bitcoin::consensus::encode::{Decodable, Error, VarInt}; - let v: VarInt = Decodable::consensus_decode(&mut r) - .map_err(|e| match e { - Error::Io(ioe) => DecodeError::from(ioe), - _ => DecodeError::InvalidValue - })?; - + let v: BigSize = Readable::read(r)?; let mut rd = FixedLengthReader::new(r, v.0); // TODO: support reply paths let mut _reply_path_bytes: Option> = Some(Vec::new());