Skip to content

Commit 187a2e9

Browse files
committed
fix: verificationState not included in unmarshal size check
- include all fields in size check - add unit test to catch regressions in this area - move constant near others so divergences are easier to track - move AddressLength constant to where Address is defined
1 parent 0ce69b1 commit 187a2e9

3 files changed

Lines changed: 32 additions & 18 deletions

File tree

node/pkg/common/chainlock.go

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ import (
1717
)
1818

1919
const (
20-
// AddressLength is the length of a normalized Wormhole address in bytes.
21-
AddressLength = 32
22-
2320
// Wormhole supports arbitrary payloads due to the variance in transaction and block sizes between chains.
2421
// However, during serialization, payload lengths are limited by Go slice length constraints and violations
2522
// of these limits can cause panics.
@@ -54,6 +51,20 @@ const (
5451
// minMsgIdLen is the minimum length of a message ID. It is used to uniquely identify
5552
// messages in the case of a duplicate message ID and is stored in the database.
5653
MinMsgIdLen = len("1/0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16/0")
54+
55+
// fixedFieldsLen is the minimum length of the fixed portion of a message publication.
56+
// It is the sum of the sizes of each of the fields plus length information for the Payload.
57+
// This is used to check that the data is long enough for the rest of the message after reading the TxID.
58+
fixedFieldsLen = 8 + // Timestamp (int64)
59+
4 + // Nonce (uint32)
60+
8 + // Sequence (uint64)
61+
1 + // ConsistencyLevel (uint8)
62+
2 + // EmitterChain (uint16)
63+
32 + // EmitterAddress (32 bytes)
64+
1 + // IsReobservation (bool)
65+
1 + // Unreliable (bool)
66+
1 + // verificationState (uint8)
67+
8 // Payload length (uint64)
5768
)
5869

5970
var (
@@ -393,7 +404,7 @@ func UnmarshalMessagePublication(data []byte) (*MessagePublication, error) {
393404
}
394405

395406
emitterAddress := vaa.Address{}
396-
if n, err := reader.Read(emitterAddress[:]); err != nil || n != AddressLength {
407+
if n, err := reader.Read(emitterAddress[:]); err != nil || n != int(vaa.AddressLength) {
397408
return nil, fmt.Errorf("failed to read emitter address [%d]: %w", n, err)
398409
}
399410
msg.EmitterAddress = emitterAddress
@@ -418,19 +429,6 @@ func UnmarshalMessagePublication(data []byte) (*MessagePublication, error) {
418429
// UnmarshalBinary implements the BinaryUnmarshaler interface for MessagePublication.
419430
func (m *MessagePublication) UnmarshalBinary(data []byte) error {
420431

421-
// fixedFieldsLen is the minimum length of the fixed portion of a message publication.
422-
// It is the sum of the sizes of each of the fields plus length information for the Payload.
423-
// This is used to check that the data is long enough for the rest of the message after reading the TxID.
424-
const fixedFieldsLen = 8 + // Timestamp (int64)
425-
4 + // Nonce (uint32)
426-
8 + // Sequence (uint64)
427-
1 + // ConsistencyLevel (uint8)
428-
2 + // EmitterChain (uint16)
429-
32 + // EmitterAddress (32 bytes)
430-
1 + // IsReobservation (bool)
431-
1 + // Unreliable (bool)
432-
8 // Payload length (uint64)
433-
434432
// Calculate minimum required length for the fixed portion
435433
// (excluding variable-length fields: TxID and Payload)
436434
if len(data) < marshaledMsgLenMin {

node/pkg/common/chainlock_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,19 @@ func TestMessagePublicationUnmarshalBinaryErrors(t *testing.T) {
135135
assert.Contains(t, inputSizeErr.Error(), "data too short")
136136
},
137137
},
138+
{
139+
name: "data too short - after non-empty TxID",
140+
errorChecker: func(t *testing.T, err error) {
141+
var inputSizeErr ErrInputSize
142+
require.ErrorAs(t, err, &inputSizeErr)
143+
assert.Contains(t, inputSizeErr.Error(), "data too short after reading TxID")
144+
},
145+
setupData: func() []byte {
146+
data := make([]byte, 1+1+fixedFieldsLen-1)
147+
data[offsetTxIDLength] = 1
148+
return data
149+
},
150+
},
138151
{
139152
name: "invalid IsReobservation boolean - value 0x02",
140153
expectedErr: ErrInvalidBinaryBool,

sdk/vaa/structs.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ type (
5353

5454
// Address is a Wormhole protocol address, it contains the native chain's address. If the address data type of a
5555
// chain is < 32bytes the value is zero-padded on the left.
56-
Address [32]byte
56+
Address [AddressLength]byte
5757

5858
// Signature of a single guardian
5959
Signature struct {
@@ -112,6 +112,9 @@ const (
112112
ConsistencyLevelSafe = uint8(201)
113113
ConsistencyLevelFinalized = uint8(202)
114114
ConsistencyLevelCustom = uint8(203)
115+
116+
// AddressLength is the length of an normalized [Address] in bytes.
117+
AddressLength uint8 = 32
115118
)
116119

117120
//nolint:unparam // error is always nil here but the return type is required to satisfy the interface.

0 commit comments

Comments
 (0)