Merge pull request #974 from Roasbeef/invoice-expiry-fix

zpay32+rpc: ensure we can encode an expiry of math.MaxUint64, limit to 1 year in seconds

rpcserver.go

@@ -2129,8 +2129,21 @@ func (r *rpcServer) AddInvoice(ctx context.Context,
 	// will be explicitly added to this payment request, which will imply
 	// the default 3600 seconds.
 	if invoice.Expiry > 0 {
-		exp := time.Duration(invoice.Expiry) * time.Second
-		options = append(options, zpay32.Expiry(exp))
+
+		// We'll ensure that the specified expiry is restricted to sane
+		// number of seconds. As a result, we'll reject an invoice with
+		// an expiry greater than 1 year.
+		maxExpiry := time.Hour * 24 * 365
+		expSeconds := invoice.Expiry
+
+		if float64(expSeconds) > maxExpiry.Seconds() {
+			return nil, fmt.Errorf("expiry of %v seconds "+
+				"greater than max expiry of %v seconds",
+				float64(expSeconds), maxExpiry.Seconds())
+		}
+
+		expiry := time.Duration(invoice.Expiry) * time.Second
+		options = append(options, zpay32.Expiry(expiry))
 	}
 
 	// If the description hash is set, then we add it do the list of options.

zpay32/invoice.go

@@ -1056,8 +1056,8 @@ func writeTaggedField(bufferBase32 *bytes.Buffer, dataType byte, data []byte) er
 
 // base32ToUint64 converts a base32 encoded number to uint64.
 func base32ToUint64(data []byte) (uint64, error) {
-	// Maximum that fits in uint64 is 64 / 5 = 12 groups.
-	if len(data) > 12 {
+	// Maximum that fits in uint64 is ceil(64 / 5) = 12 groups.
+	if len(data) > 13 {
 		return 0, fmt.Errorf("cannot parse data of length %d as uint64",
 			len(data))
 	}
@@ -1077,9 +1077,9 @@ func uint64ToBase32(num uint64) []byte {
 		return []byte{0}
 	}
 
-	// To fit an uint64, we need at most is 64 / 5 = 12 groups.
-	arr := make([]byte, 12)
-	i := 12
+	// To fit an uint64, we need at most is ceil(64 / 5) = 13 groups.
+	arr := make([]byte, 13)
+	i := 13
 	for num > 0 {
 		i--
 		arr[i] = byte(num & uint64(31)) // 0b11111 in binary

zpay32/invoice_internal_test.go

@@ -2,6 +2,7 @@ package zpay32
 
 import (
 	"encoding/binary"
+	"math"
 	"reflect"
 	"testing"
 	"time"
@@ -527,7 +528,11 @@ func TestParseExpiry(t *testing.T) {
 			result: &testExpiry60,
 		},
 		{
-			data:  []byte{0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc},
+			data: []byte{
+				0x0, 0x1, 0x2, 0x3, 0x4, 0x5,
+				0x6, 0x7, 0x8, 0x9, 0xa, 0xb,
+				0xc, 0x3,
+			},
 			valid: false, // data too long
 		},
 	}
@@ -547,7 +552,8 @@ func TestParseExpiry(t *testing.T) {
 	}
 }
 
-// TestParseMinFinalCLTVExpiry checks that the minFinalCLTVExpiry is properly parsed.
+// TestParseMinFinalCLTVExpiry checks that the minFinalCLTVExpiry is properly
+// parsed.
 func TestParseMinFinalCLTVExpiry(t *testing.T) {
 	t.Parallel()
 
@@ -567,12 +573,20 @@ func TestParseMinFinalCLTVExpiry(t *testing.T) {
 			result: 60,
 		},
 		{
-			data:   []byte{0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc},
+			data: []byte{
+				0x1, 0x2, 0x3, 0x4, 0x5,
+				0x6, 0x7, 0x8, 0x9, 0xa,
+				0xb, 0xc,
+			},
 			valid:  true,
 			result: 38390726480144748,
 		},
 		{
-			data:  []byte{0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc},
+			data: []byte{
+				0x0, 0x1, 0x2, 0x3, 0x4, 0x5,
+				0x6, 0x7, 0x8, 0x9, 0xa, 0xb,
+				0xc, 0x94,
+			},
 			valid: false, // data too long
 		},
 	}
@@ -592,6 +606,26 @@ func TestParseMinFinalCLTVExpiry(t *testing.T) {
 	}
 }
 
+// TestParseMinFinalCLTVExpiry tests that were able to properly encode/decode
+// the math.MaxUint64 integer without panicking.
+func TestParseMaxUint64Expiry(t *testing.T) {
+	t.Parallel()
+
+	expiry := uint64(math.MaxUint64)
+
+	expiryBytes := uint64ToBase32(expiry)
+
+	expiryReParse, err := base32ToUint64(expiryBytes)
+	if err != nil {
+		t.Fatalf("unable to parse uint64: %v", err)
+	}
+
+	if expiryReParse != expiry {
+		t.Fatalf("wrong expiry: expected %v got %v", expiry,
+			expiryReParse)
+	}
+}
+
 // TestParseFallbackAddr checks that the fallback address is properly parsed.
 func TestParseFallbackAddr(t *testing.T) {
 	t.Parallel()