From ba554d9d877909d12e884556b27d44e75e559db7 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Tue, 17 Feb 2026 02:05:36 -0600 Subject: [PATCH 1/2] wire: Add a couple of missing err stringer tests. This adds a couple of tests to ensure a couple of the newer error codes produce the expected human-readable output that were missed when adding them. It also adds a new define to the enum to count how many there are along with a test to detect missing entries. --- wire/error.go | 4 ++++ wire/error_test.go | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/wire/error.go b/wire/error.go index dd7fbd1cc1..020bb074e7 100644 --- a/wire/error.go +++ b/wire/error.go @@ -161,6 +161,10 @@ const ( // ErrUnknownNetAddrType is returned when a network address type is not // recognized or supported. ErrUnknownNetAddrType + + // numErrorCodes is the total number of error codes defined above. This + // entry MUST be the last entry in the enum. + numErrorCodes ) // Map of ErrorCode values back to their constant names for pretty printing. diff --git a/wire/error_test.go b/wire/error_test.go index 4520425041..75c1dad38f 100644 --- a/wire/error_test.go +++ b/wire/error_test.go @@ -55,10 +55,18 @@ func TestMessageErrorCodeStringer(t *testing.T) { {ErrTooManyMixPairReqUTXOs, "ErrTooManyMixPairReqUTXOs"}, {ErrTooManyPrevMixMsgs, "ErrTooManyPrevMixMsgs"}, {ErrTooManyCFilters, "ErrTooManyCFilters"}, + {ErrTooFewAddrs, "ErrTooFewAddrs"}, + {ErrUnknownNetAddrType, "ErrUnknownNetAddrType"}, {0xffff, "Unknown ErrorCode (65535)"}, } + // Detect additional defines that don't have the stringer added. + if len(tests)-1 != int(numErrorCodes) { + t.Fatal("It appears an error code was added without adding an " + + "associated stringer test") + } + t.Logf("Running %d tests", len(tests)) for i, test := range tests { result := test.in.String() From af4aaac0281ce7639be1fb049015b03e506add0b Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Tue, 17 Feb 2026 02:29:05 -0600 Subject: [PATCH 2/2] wire: Clamp max allowed timestamp values. This modifies the code that deals with serializing and deserializing the newly added addrv2 netaddress timestamps uint64s to be more restrictive and reject any values that result in times that require special handling for comparisons. While here, it also does the same for int64 timestamps even though those cases are already handled otherwise. This clamping behavior is not strictly required since code that deals with the timestamps later is careful to avoid bad timestamps in general, but it safer to just reject them at the protocol level so even if code elsewhere is not taking extra precautions there would still not be any potential issues. --- wire/common.go | 53 +++++++++++++++++++++++++++++++++++------- wire/error.go | 5 ++++ wire/error_test.go | 1 + wire/msgaddrv2_test.go | 16 +++++++++++-- wire/msgversion.go | 13 ++++------- 5 files changed, 69 insertions(+), 19 deletions(-) diff --git a/wire/common.go b/wire/common.go index 4f9dd35aba..75728cd4fc 100644 --- a/wire/common.go +++ b/wire/common.go @@ -32,6 +32,10 @@ const ( // strictAsciiRangeUpper is the upper limit of the strict ASCII range. strictAsciiRangeUpper = 0x7e + + // unixToInternal is the number of seconds between year 1 of the Go time + // value and the unix epoch. + unixToInternal = 62135596800 ) var ( @@ -150,14 +154,17 @@ type uint32Time time.Time // int64Time represents a unix timestamp encoded with an int64. It is used as // a way to signal the readElement function how to decode a timestamp into a Go -// time.Time since it is otherwise ambiguous. +// time.Time since it is otherwise ambiguous. The value is rejected if it is +// larger than the maximum usable seconds for a Go time value for worry-free +// comparisons. type int64Time time.Time // uint64Time represents a unix timestamp encoded with a uint64. It is used as // a way to signal the readElement function how to decode a timestamp into a Go // time.Time since it is otherwise ambiguous. The uint64 value is rejected if -// it is larger than the maximum int64 value since it would overflow when -// converted to an int64 for the time.Unix call. +// it is larger than the maximum usable seconds for a Go time value for +// worry-free comparisons which also has the side effect of preventing overflow +// when converting to an int64 for the time.Unix call. type uint64Time time.Time // readElement reads the next sequence of bytes from r using little endian @@ -241,6 +248,13 @@ func readElement(r io.Reader, element interface{}) error { if err != nil { return err } + + // Reject timestamps that would overflow the maximum usable number of + // seconds for worry-free comparisons. + if rv > math.MaxInt64-unixToInternal { + const str = "timestamp exceeds maximum allowed value" + return messageError("readElement", ErrInvalidTimestamp, str) + } *e = int64Time(time.Unix(int64(rv), 0)) return nil @@ -249,10 +263,12 @@ func readElement(r io.Reader, element interface{}) error { if err != nil { return err } - // Reject timestamps that would overflow when converted to int64. - if rv > math.MaxInt64 { - return messageError("readElement", ErrInvalidMsg, - "timestamp exceeds maximum allowed value") + + // Reject timestamps that would overflow the maximum usable number of + // seconds for worry-free comparisons. + if rv > math.MaxInt64-unixToInternal { + const str = "timestamp exceeds maximum allowed value" + return messageError("readElement", ErrInvalidTimestamp, str) } *e = uint64Time(time.Unix(int64(rv), 0)) return nil @@ -534,8 +550,29 @@ func writeElement(w io.Writer, element interface{}) error { } return nil + case *int64Time: + // Reject timestamps that would overflow the maximum usable number of + // seconds for worry-free comparisons. + secs := uint64(time.Time(*e).Unix()) + if secs > math.MaxInt64-unixToInternal { + const str = "timestamp exceeds maximum allowed value" + return messageError("writeElement", ErrInvalidTimestamp, str) + } + err := writeUint64LE(w, secs) + if err != nil { + return err + } + return nil + case *uint64Time: - err := writeUint64LE(w, uint64(time.Time(*e).Unix())) + // Reject timestamps that would overflow the maximum usable number of + // seconds for worry-free comparisons. + secs := uint64(time.Time(*e).Unix()) + if secs > math.MaxInt64-unixToInternal { + const str = "timestamp exceeds maximum allowed value" + return messageError("writeElement", ErrInvalidTimestamp, str) + } + err := writeUint64LE(w, secs) if err != nil { return err } diff --git a/wire/error.go b/wire/error.go index 020bb074e7..80fcad5e3b 100644 --- a/wire/error.go +++ b/wire/error.go @@ -162,6 +162,10 @@ const ( // recognized or supported. ErrUnknownNetAddrType + // ErrInvalidTimestamp is returned when a message that involves a timestamp + // is not in the allowable range. + ErrInvalidTimestamp + // numErrorCodes is the total number of error codes defined above. This // entry MUST be the last entry in the enum. numErrorCodes @@ -207,6 +211,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrTooManyCFilters: "ErrTooManyCFilters", ErrTooFewAddrs: "ErrTooFewAddrs", ErrUnknownNetAddrType: "ErrUnknownNetAddrType", + ErrInvalidTimestamp: "ErrInvalidTimestamp", } // String returns the ErrorCode as a human-readable name. diff --git a/wire/error_test.go b/wire/error_test.go index 75c1dad38f..035d407f72 100644 --- a/wire/error_test.go +++ b/wire/error_test.go @@ -57,6 +57,7 @@ func TestMessageErrorCodeStringer(t *testing.T) { {ErrTooManyCFilters, "ErrTooManyCFilters"}, {ErrTooFewAddrs, "ErrTooFewAddrs"}, {ErrUnknownNetAddrType, "ErrUnknownNetAddrType"}, + {ErrInvalidTimestamp, "ErrInvalidTimestamp"}, {0xffff, "Unknown ErrorCode (65535)"}, } diff --git a/wire/msgaddrv2_test.go b/wire/msgaddrv2_test.go index 664480426f..bd83d2ba5c 100644 --- a/wire/msgaddrv2_test.go +++ b/wire/msgaddrv2_test.go @@ -8,6 +8,7 @@ import ( "bytes" "errors" "io" + "math" "reflect" "testing" "time" @@ -260,14 +261,14 @@ func TestAddrV2BtcDecode(t *testing.T) { pver: pver, wireBytes: []byte{ 0x01, // Varint address count - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, // Timestamp (MaxInt64+1) + 0x00, 0x09, 0x6e, 0x88, 0xf1, 0xff, 0xff, 0x7f, // Timestamp (max+1) 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Services 0x01, // Type (IPv4) 0x7f, 0x00, 0x00, 0x01, // IP 0x8d, 0x20, // Port 8333 (little-endian) }, wantAddrs: nil, - wantErr: ErrInvalidMsg, + wantErr: ErrInvalidTimestamp, }, { name: "message with valid types and unknown type", pver: pver, @@ -391,6 +392,17 @@ func TestAddrV2BtcEncode(t *testing.T) { Port: 8333, }}, wantErr: ErrUnknownNetAddrType, + }, { + name: "message with invalid timestamp", + pver: pver, + addrs: []NetAddressV2{{ + Timestamp: time.Unix(math.MaxInt64-unixToInternal+1, 0), + Services: SFNodeNetwork, + Type: IPv4Address, + EncodedAddr: []byte{0x7f, 0x00, 0x00, 0x01}, + Port: 8333, + }}, + wantErr: ErrInvalidTimestamp, }} for _, test := range tests { diff --git a/wire/msgversion.go b/wire/msgversion.go index 9773634ea5..2db054dfaf 100644 --- a/wire/msgversion.go +++ b/wire/msgversion.go @@ -157,13 +157,8 @@ func (msg *MsgVersion) BtcEncode(w io.Writer, pver uint32) error { return err } - var elems struct { - ts int64 - relayTx bool - } - elems.ts = msg.Timestamp.Unix() - - err = writeElements(w, &msg.ProtocolVersion, &msg.Services, &elems.ts) + err = writeElements(w, &msg.ProtocolVersion, &msg.Services, + (*int64Time)(&msg.Timestamp)) if err != nil { return err } @@ -193,8 +188,8 @@ func (msg *MsgVersion) BtcEncode(w io.Writer, pver uint32) error { return err } - elems.relayTx = !msg.DisableRelayTx - return writeElement(w, &elems.relayTx) + var relayTx = !msg.DisableRelayTx + return writeElement(w, &relayTx) } // Command returns the protocol command string for the message. This is part