Skip to content

Commit 3386c4b

Browse files
slanesukejkczyz
authored and
TheBlueMatt
committed
Validate amount_msats against invreq amount
Add a check to ensure that the amount_msats in an invoice matches the amount_msats specified in the invoice_request or offer (or refund). Reject the invoice as invalid if there is a mismatch between these amounts. Otherwise, an invoice may be paid with an amount greater than the requested amount. Co-authored-by: Ian Slane <[email protected]> Co-authored-by: Jeffrey Czyz <[email protected]>
1 parent 8202250 commit 3386c4b

File tree

5 files changed

+144
-11
lines changed

5 files changed

+144
-11
lines changed

‎lightning/src/ln/offers_tests.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() {
566566
human_readable_name: None,
567567
},
568568
});
569-
assert_eq!(invoice_request.amount_msats(), None);
569+
assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
570570
assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
571571
assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
572572

@@ -727,7 +727,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() {
727727
human_readable_name: None,
728728
},
729729
});
730-
assert_eq!(invoice_request.amount_msats(), None);
730+
assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
731731
assert_ne!(invoice_request.payer_signing_pubkey(), bob_id);
732732
assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id));
733733

@@ -1116,7 +1116,7 @@ fn creates_and_pays_for_offer_with_retry() {
11161116
human_readable_name: None,
11171117
},
11181118
});
1119-
assert_eq!(invoice_request.amount_msats(), None);
1119+
assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
11201120
assert_ne!(invoice_request.payer_signing_pubkey(), bob_id);
11211121
assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id));
11221122
let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
@@ -1411,7 +1411,7 @@ fn fails_authentication_when_handling_invoice_request() {
14111411
alice.onion_messenger.handle_onion_message(david_id, &onion_message);
14121412

14131413
let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
1414-
assert_eq!(invoice_request.amount_msats(), None);
1414+
assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
14151415
assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
14161416
assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
14171417

@@ -1441,7 +1441,7 @@ fn fails_authentication_when_handling_invoice_request() {
14411441
alice.onion_messenger.handle_onion_message(bob_id, &onion_message);
14421442

14431443
let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
1444-
assert_eq!(invoice_request.amount_msats(), None);
1444+
assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
14451445
assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
14461446
assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
14471447

@@ -1543,7 +1543,7 @@ fn fails_authentication_when_handling_invoice_for_offer() {
15431543
alice.onion_messenger.handle_onion_message(bob_id, &onion_message);
15441544

15451545
let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
1546-
assert_eq!(invoice_request.amount_msats(), None);
1546+
assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
15471547
assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
15481548
assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
15491549

‎lightning/src/offers/invoice.rs

+76-1
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ macro_rules! invoice_builder_methods { (
342342
pub(crate) fn amount_msats(
343343
invoice_request: &InvoiceRequest
344344
) -> Result<u64, Bolt12SemanticError> {
345-
match invoice_request.amount_msats() {
345+
match invoice_request.contents.inner.amount_msats() {
346346
Some(amount_msats) => Ok(amount_msats),
347347
None => match invoice_request.contents.inner.offer.amount() {
348348
Some(Amount::Bitcoin { amount_msats }) => {
@@ -1531,6 +1531,11 @@ impl TryFrom<PartialInvoiceTlvStream> for InvoiceContents {
15311531
experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
15321532
)
15331533
)?;
1534+
1535+
if amount_msats != refund.amount_msats() {
1536+
return Err(Bolt12SemanticError::InvalidAmount);
1537+
}
1538+
15341539
Ok(InvoiceContents::ForRefund { refund, fields })
15351540
} else {
15361541
let invoice_request = InvoiceRequestContents::try_from(
@@ -1539,6 +1544,13 @@ impl TryFrom<PartialInvoiceTlvStream> for InvoiceContents {
15391544
experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
15401545
)
15411546
)?;
1547+
1548+
if let Some(requested_amount_msats) = invoice_request.amount_msats() {
1549+
if amount_msats != requested_amount_msats {
1550+
return Err(Bolt12SemanticError::InvalidAmount);
1551+
}
1552+
}
1553+
15421554
Ok(InvoiceContents::ForOffer { invoice_request, fields })
15431555
}
15441556
}
@@ -2707,6 +2719,69 @@ mod tests {
27072719
}
27082720
}
27092721

2722+
#[test]
2723+
fn fails_parsing_invoice_with_wrong_amount() {
2724+
let expanded_key = ExpandedKey::new([42; 32]);
2725+
let entropy = FixedEntropy {};
2726+
let nonce = Nonce::from_entropy_source(&entropy);
2727+
let secp_ctx = Secp256k1::new();
2728+
let payment_id = PaymentId([1; 32]);
2729+
2730+
let invoice = OfferBuilder::new(recipient_pubkey())
2731+
.amount_msats(1000)
2732+
.build().unwrap()
2733+
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
2734+
.build_and_sign().unwrap()
2735+
.respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap()
2736+
.amount_msats_unchecked(2000)
2737+
.build().unwrap()
2738+
.sign(recipient_sign).unwrap();
2739+
2740+
let mut buffer = Vec::new();
2741+
invoice.write(&mut buffer).unwrap();
2742+
2743+
match Bolt12Invoice::try_from(buffer) {
2744+
Ok(_) => panic!("expected error"),
2745+
Err(e) => assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)),
2746+
}
2747+
2748+
let invoice = OfferBuilder::new(recipient_pubkey())
2749+
.amount_msats(1000)
2750+
.build().unwrap()
2751+
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
2752+
.amount_msats(1000).unwrap()
2753+
.build_and_sign().unwrap()
2754+
.respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap()
2755+
.amount_msats_unchecked(2000)
2756+
.build().unwrap()
2757+
.sign(recipient_sign).unwrap();
2758+
2759+
let mut buffer = Vec::new();
2760+
invoice.write(&mut buffer).unwrap();
2761+
2762+
match Bolt12Invoice::try_from(buffer) {
2763+
Ok(_) => panic!("expected error"),
2764+
Err(e) => assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)),
2765+
}
2766+
2767+
let invoice = RefundBuilder::new(vec![1; 32], payer_pubkey(), 1000).unwrap()
2768+
.build().unwrap()
2769+
.respond_using_derived_keys_no_std(
2770+
payment_paths(), payment_hash(), now(), &expanded_key, &entropy
2771+
)
2772+
.unwrap()
2773+
.amount_msats_unchecked(2000)
2774+
.build_and_sign(&secp_ctx).unwrap();
2775+
2776+
let mut buffer = Vec::new();
2777+
invoice.write(&mut buffer).unwrap();
2778+
2779+
match Bolt12Invoice::try_from(buffer) {
2780+
Ok(_) => panic!("expected error"),
2781+
Err(e) => assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)),
2782+
}
2783+
}
2784+
27102785
#[test]
27112786
fn fails_parsing_invoice_without_signature() {
27122787
let expanded_key = ExpandedKey::new([42; 32]);

‎lightning/src/offers/invoice_macros.rs

+8
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ macro_rules! invoice_builder_methods_test { (
8787
$self: ident, $self_type: ty, $invoice_fields: expr, $return_type: ty, $return_value: expr
8888
$(, $self_mut: tt)?
8989
) => {
90+
#[cfg_attr(c_bindings, allow(dead_code))]
91+
pub(crate) fn amount_msats_unchecked(
92+
$($self_mut)* $self: $self_type, amount_msats: u64,
93+
) -> $return_type {
94+
$invoice_fields.amount_msats = amount_msats;
95+
$return_value
96+
}
97+
9098
#[cfg_attr(c_bindings, allow(dead_code))]
9199
pub(crate) fn features_unchecked(
92100
$($self_mut)* $self: $self_type, features: Bolt12InvoiceFeatures

‎lightning/src/offers/invoice_request.rs

+53-3
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
7979
use crate::ln::msgs::DecodeError;
8080
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
8181
use crate::offers::nonce::Nonce;
82-
use crate::offers::offer::{EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
82+
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
8383
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
8484
use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef};
8585
use crate::offers::signer::{Metadata, MetadataMaterial};
@@ -974,7 +974,15 @@ impl InvoiceRequestContents {
974974
}
975975

976976
pub(super) fn amount_msats(&self) -> Option<u64> {
977-
self.inner.amount_msats
977+
self.inner
978+
.amount_msats()
979+
.or_else(|| match self.inner.offer.amount() {
980+
Some(Amount::Bitcoin { amount_msats }) => {
981+
Some(amount_msats.saturating_mul(self.quantity().unwrap_or(1)))
982+
},
983+
Some(Amount::Currency { .. }) => None,
984+
None => { debug_assert!(false); None},
985+
})
978986
}
979987

980988
pub(super) fn features(&self) -> &InvoiceRequestFeatures {
@@ -1015,6 +1023,10 @@ impl InvoiceRequestContentsWithoutPayerSigningPubkey {
10151023
self.chain.unwrap_or_else(|| self.offer.implied_chain())
10161024
}
10171025

1026+
pub(super) fn amount_msats(&self) -> Option<u64> {
1027+
self.amount_msats
1028+
}
1029+
10181030
pub(super) fn as_tlv_stream(&self) -> PartialInvoiceRequestTlvStreamRef {
10191031
let payer = PayerTlvStreamRef {
10201032
metadata: self.payer.0.as_bytes(),
@@ -1381,7 +1393,7 @@ mod tests {
13811393
assert_eq!(invoice_request.supported_quantity(), Quantity::One);
13821394
assert_eq!(invoice_request.issuer_signing_pubkey(), Some(recipient_pubkey()));
13831395
assert_eq!(invoice_request.chain(), ChainHash::using_genesis_block(Network::Bitcoin));
1384-
assert_eq!(invoice_request.amount_msats(), None);
1396+
assert_eq!(invoice_request.amount_msats(), Some(1000));
13851397
assert_eq!(invoice_request.invoice_request_features(), &InvoiceRequestFeatures::empty());
13861398
assert_eq!(invoice_request.quantity(), None);
13871399
assert_eq!(invoice_request.payer_note(), None);
@@ -1748,6 +1760,44 @@ mod tests {
17481760
}
17491761
}
17501762

1763+
#[test]
1764+
fn builds_invoice_request_without_amount() {
1765+
let expanded_key = ExpandedKey::new([42; 32]);
1766+
let entropy = FixedEntropy {};
1767+
let nonce = Nonce::from_entropy_source(&entropy);
1768+
let secp_ctx = Secp256k1::new();
1769+
let payment_id = PaymentId([1; 32]);
1770+
1771+
let invoice_request = OfferBuilder::new(recipient_pubkey())
1772+
.amount_msats(1000)
1773+
.build().unwrap()
1774+
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
1775+
.build_and_sign().unwrap();
1776+
let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream();
1777+
assert_eq!(invoice_request.amount_msats(), Some(1000));
1778+
assert_eq!(tlv_stream.amount, None);
1779+
1780+
let invoice_request = OfferBuilder::new(recipient_pubkey())
1781+
.amount_msats(1000)
1782+
.supported_quantity(Quantity::Unbounded)
1783+
.build().unwrap()
1784+
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
1785+
.quantity(2).unwrap()
1786+
.build_and_sign().unwrap();
1787+
let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream();
1788+
assert_eq!(invoice_request.amount_msats(), Some(2000));
1789+
assert_eq!(tlv_stream.amount, None);
1790+
1791+
let invoice_request = OfferBuilder::new(recipient_pubkey())
1792+
.amount(Amount::Currency { iso4217_code: *b"USD", amount: 10 })
1793+
.build_unchecked()
1794+
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
1795+
.build_unchecked_and_sign();
1796+
let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream();
1797+
assert_eq!(invoice_request.amount_msats(), None);
1798+
assert_eq!(tlv_stream.amount, None);
1799+
}
1800+
17511801
#[test]
17521802
fn builds_invoice_request_with_features() {
17531803
let expanded_key = ExpandedKey::new([42; 32]);

‎lightning/src/offers/parse.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ pub enum Bolt12SemanticError {
147147
UnexpectedChain,
148148
/// An amount was expected but was missing.
149149
MissingAmount,
150-
/// The amount exceeded the total bitcoin supply.
150+
/// The amount exceeded the total bitcoin supply or didn't match an expected amount.
151151
InvalidAmount,
152152
/// An amount was provided but was not sufficient in value.
153153
InsufficientAmount,

0 commit comments

Comments
 (0)