lnwallet/channel: fix crash on receiving too few HTLC sigs

This commit fixes an out of bounds error that would occur in the case
where we received a new commitment where the accompanying HTLC sigs were
too few. Now we'll just reject such an commitment.

A test exercising the behavior is also added.
This commit is contained in:
Johan T. Halseth 2018-03-22 12:52:12 +01:00
parent 807b84ee63
commit 241c79397f
No known key found for this signature in database
GPG Key ID: 15BAADA29DA20D26
2 changed files with 83 additions and 0 deletions

@ -3532,6 +3532,12 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment,
return sigHash, nil
}
// Make sure there are more signatures left.
if i >= len(htlcSigs) {
return nil, fmt.Errorf("not enough HTLC " +
"signatures.")
}
// With the sighash generated, we'll also store the
// signature so it can be written to disk if this state
// is valid.
@ -3578,6 +3584,12 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment,
return sigHash, nil
}
// Make sure there are more signatures left.
if i >= len(htlcSigs) {
return nil, fmt.Errorf("not enough HTLC " +
"signatures.")
}
// With the sighash generated, we'll also store the
// signature so it can be written to disk if this state
// is valid.

@ -1461,6 +1461,77 @@ func TestHTLCDustLimit(t *testing.T) {
}
}
// TestHTLCSigNumber tests that a received commitment is only accepted if it
// comes with the exact number of valid HTLC signatures.
func TestHTLCSigNumber(t *testing.T) {
t.Parallel()
// createChanWithHTLC is a helper method that sets ut two channels, and
// adds HTLCs with the passed values to the channels.
createChanWithHTLC := func(htlcValues ...btcutil.Amount) (
*LightningChannel, *LightningChannel, func()) {
// Create a test channel funded evenly with Alice having 5 BTC,
// and Bob having 5 BTC. Alice's dustlimit is 200 sat, while
// Bob has 1300 sat.
aliceChannel, bobChannel, cleanUp, err := createTestChannels(3)
if err != nil {
t.Fatalf("unable to create test channels: %v", err)
}
for i, htlcSat := range htlcValues {
htlcMsat := lnwire.NewMSatFromSatoshis(htlcSat)
htlc, _ := createHTLC(i, htlcMsat)
_, err := aliceChannel.AddHTLC(htlc, nil)
if err != nil {
t.Fatalf("alice unable to add htlc: %v", err)
}
_, err = bobChannel.ReceiveHTLC(htlc)
if err != nil {
t.Fatalf("bob unable to receive htlc: %v", err)
}
}
return aliceChannel, bobChannel, cleanUp
}
// Calculate two values that will be below and above Bob's dust limit.
estimator := &StaticFeeEstimator{24}
feePerVSize, err := estimator.EstimateFeePerVSize(1)
if err != nil {
t.Fatalf("unable to get fee: %v", err)
}
feePerKw := feePerVSize.FeePerKWeight()
aboveDust := btcutil.Amount(1400) + htlcSuccessFee(feePerKw)
// ===================================================================
// Test that Bob will reject a commitment if Alice doesn't send enough
// HTLC signatures.
// ===================================================================
aliceChannel, bobChannel, cleanUp := createChanWithHTLC(aboveDust,
aboveDust)
defer cleanUp()
aliceSig, aliceHtlcSigs, err := aliceChannel.SignNextCommitment()
if err != nil {
t.Fatalf("Error signing next commitment: %v", err)
}
if len(aliceHtlcSigs) != 2 {
t.Fatalf("expected 2 htlc sig, instead got %v",
len(aliceHtlcSigs))
}
// Now discard one signature from the htlcSig slice. Bob should reject
// the commitment because of this.
err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs[1:])
if err == nil {
t.Fatalf("Expected Bob to reject signatures")
}
}
// TestChannelBalanceDustLimit tests the condition when the remaining balance
// for one of the channel participants is so small as to be considered dust. In
// this case, the output for that participant is removed and all funds (minus