X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=8daea4d7548fda4d1f72d50b4e3f46774d404f8a;hb=refs%2Fheads%2F2022-01-fuzz-overflow;hp=19f73ba65450017fd63e0b6ce1d3eededc8d21a4;hpb=803d6b6e2fb8664a671c92156e6b1fb11aba0ab1;p=rust-lightning

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 19f73ba6..8daea4d7 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -870,14 +870,6 @@ impl<Signer: Sign> Channel<Signer> {
 	fn check_remote_fee<F: Deref>(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError>
 		where F::Target: FeeEstimator
 	{
-		let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-		// Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
-		// occasional issues with feerate disagreements between an initiator that wants a feerate
-		// of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
-		// sat/kw before the comparison here.
-		if feerate_per_kw + 250 < lower_limit {
-			return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
-		}
 		// We only bound the fee updates on the upper side to prevent completely absurd feerates,
 		// always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
 		// We generally don't care too much if they set the feerate to something very high, but it
@@ -887,6 +879,14 @@ impl<Signer: Sign> Channel<Signer> {
 		if feerate_per_kw as u64 > upper_limit {
 			return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
 		}
+		let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+		// Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
+		// occasional issues with feerate disagreements between an initiator that wants a feerate
+		// of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
+		// sat/kw before the comparison here.
+		if feerate_per_kw + 250 < lower_limit {
+			return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
+		}
 		Ok(())
 	}
 
@@ -5859,6 +5859,13 @@ mod tests {
 		        "MAX_FUNDING_SATOSHIS is greater than all satoshis in existence");
 	}
 
+	#[test]
+	fn test_no_fee_check_overflow() {
+		// Previously, calling `check_remote_fee` with a fee of 0xffffffff would overflow in
+		// arithmetic, causing a panic with debug assertions enabled.
+		assert!(Channel::<InMemorySigner>::check_remote_fee(&&TestFeeEstimator { fee_est: 42 }, u32::max_value()).is_err());
+	}
+
 	struct Keys {
 		signer: InMemorySigner,
 	}
@@ -5993,7 +6000,7 @@ mod tests {
 				first_hop_htlc_msat: 548,
 				payment_id: PaymentId([42; 32]),
 				payment_secret: None,
-				payee: None,
+				payment_params: None,
 			}
 		});