Skip to content

Commit

Permalink
Fix bug in onion message payload decode
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
valentinewallace committed Aug 24, 2022
1 parent dceca5b commit 351349c
Showing 1 changed file with 2 additions and 8 deletions.
10 changes: 2 additions & 8 deletions lightning/src/onion_message/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -161,13 +161,7 @@ impl Writeable for (Payload, [u8; 32]) {
// Uses the provided secret to simultaneously decode and decrypt the control TLVs.
impl ReadableArgs<SharedSecret> for Payload {
fn read<R: Read>(mut r: &mut R, encrypted_tlvs_ss: SharedSecret) -> Result<Self, DecodeError> {
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<Vec<u8>> = Some(Vec::new());
Expand Down

0 comments on commit 351349c

Please sign in to comment.