diff --git a/universalClient/chains/svm/tx_builder.go b/universalClient/chains/svm/tx_builder.go index b8ecdbce..7d26f928 100644 --- a/universalClient/chains/svm/tx_builder.go +++ b/universalClient/chains/svm/tx_builder.go @@ -104,8 +104,8 @@ type TxBuilder struct { gatewayAddress solana.PublicKey nodeHome string logger zerolog.Logger - protocolALT solana.PublicKey // Protocol ALT pubkey (zero if not configured) - tokenALTs map[solana.PublicKey]solana.PublicKey // mint pubkey → token ALT pubkey + protocolALT solana.PublicKey // Protocol ALT pubkey (zero if not configured) + tokenALTs map[solana.PublicKey]solana.PublicKey // mint pubkey → token ALT pubkey } // NewTxBuilder creates a new Solana transaction builder. @@ -367,17 +367,10 @@ func (tb *TxBuilder) GetOutboundSigningRequest( instructionID = fallbackID } - // Validate instruction_id - if instructionID != 1 && instructionID != 2 { - return nil, fmt.Errorf("invalid instruction_id: %d (expected 1=withdraw or 2=execute)", instructionID) - } - - // Validate mode-specific constraints per integration guide + // Mode-specific semantic checks. Shape (instruction_id ∈ {1,2}, withdraw ⇒ + // no accounts/ix_data) is already guaranteed by decodePayload + determineInstructionID. switch instructionID { case 1: // Withdraw mode - if len(accounts) > 0 || len(ixData) > 0 { - return nil, fmt.Errorf("withdraw mode: accounts and ixData must be empty") - } if amount.Uint64() == 0 { return nil, fmt.Errorf("withdraw mode: amount must be > 0") } @@ -693,10 +686,6 @@ func (tb *TxBuilder) BuildOutboundTransaction( } instructionID = fallbackID } - - if instructionID != 1 && instructionID != 2 { - return nil, 0, fmt.Errorf("invalid instruction_id: %d", instructionID) - } } // --- Derive PDAs --- @@ -1153,63 +1142,77 @@ func (tb *TxBuilder) loadRelayerKeypair() (solana.PrivateKey, error) { // The payload is built off-chain and encodes the operation type plus any // target program data needed for execution: // -// [u32 BE] accounts_count — how many accounts the target program needs -// [33 bytes] × N accounts — each is [pubkey(32) + is_writable(1)] -// [u32 BE] ix_data_len — length of the instruction data for the target program -// [N bytes] ix_data — the raw instruction data to pass to the target program -// [u8] instruction_id — 1=withdraw, 2=execute -// [32 bytes] target_program — the Solana program to invoke +// bytes [0 .. 4) accountsCount u32 — number of CPI accounts (N) +// bytes [4 .. 4+33N) accounts N × {pubkey[32], isWritable[1]} +// bytes [4+33N .. 8+33N) ixDataLen u32 — length of ix_data in bytes (M) +// bytes [8+33N .. 8+33N+M) ixData M raw bytes for the target program +// byte [8+33N+M] instructionID u8 — 1=withdraw, 2=execute +// bytes [9+33N+M .. 41+33N+M) targetProgram 32 bytes — the Solana program to invoke // // For withdraw (instruction_id=1): accounts_count=0, ix_data_len=0 -// For execute (instruction_id=2): accounts and ix_data contain CPI data +// For execute (instruction_id=2): accounts and ix_data contain CPI data func decodePayload(payload []byte) ([]GatewayAccountMeta, []byte, uint8, [32]byte, error) { + const ( + sizeAccountsCount = 4 + sizeAccount = 33 // 32-byte pubkey + 1-byte is_writable + sizeIxDataLen = 4 + sizeInstructionID = 1 + sizeTargetProgram = 32 + + minPayloadLen = sizeAccountsCount + sizeIxDataLen + sizeInstructionID + sizeTargetProgram + ) + var targetProgram [32]byte - // Minimum payload: accounts_count(4) + ix_data_len(4) + instruction_id(1) + target_program(32) = 41 - if len(payload) < 41 { - return nil, nil, 0, targetProgram, fmt.Errorf("payload too short: %d bytes (minimum 41)", len(payload)) - } + // --- Validate structure and bounds --- - offset := 0 + if len(payload) < minPayloadLen { + return nil, nil, 0, targetProgram, fmt.Errorf("payload too short: %d bytes (minimum %d)", len(payload), minPayloadLen) + } - accountsCount := binary.BigEndian.Uint32(payload[offset : offset+4]) - offset += 4 + accountsCount := binary.BigEndian.Uint32(payload[0:sizeAccountsCount]) - accounts := make([]GatewayAccountMeta, accountsCount) - for i := uint32(0); i < accountsCount; i++ { - if offset+33 > len(payload) { - return nil, nil, 0, targetProgram, fmt.Errorf("payload too short for account %d", i) - } - var pubkey [32]byte - copy(pubkey[:], payload[offset:offset+32]) - isWritable := payload[offset+32] == 1 - accounts[i] = GatewayAccountMeta{Pubkey: pubkey, IsWritable: isWritable} - offset += 33 + ixDataLenOff := uint64(sizeAccountsCount) + uint64(accountsCount)*sizeAccount + if ixDataLenOff+sizeIxDataLen > uint64(len(payload)) { + return nil, nil, 0, targetProgram, fmt.Errorf("payload too short for %d accounts: need %d bytes through ix_data length, have %d", accountsCount, ixDataLenOff+sizeIxDataLen, len(payload)) } + // Past this check ixDataLenOff is bounded by len(payload), so int conversion is safe. + ixDataLen := binary.BigEndian.Uint32(payload[ixDataLenOff : ixDataLenOff+sizeIxDataLen]) - if offset+4 > len(payload) { - return nil, nil, 0, targetProgram, fmt.Errorf("payload too short for ix_data length") + // Payload must be consumed exactly — no truncation, no trailing bytes. + expectedLen := ixDataLenOff + sizeIxDataLen + uint64(ixDataLen) + sizeInstructionID + sizeTargetProgram + switch { + case uint64(len(payload)) < expectedLen: + return nil, nil, 0, targetProgram, fmt.Errorf("payload too short: expected %d bytes, got %d (accountsCount=%d, ixDataLen=%d)", expectedLen, len(payload), accountsCount, ixDataLen) + case uint64(len(payload)) > expectedLen: + return nil, nil, 0, targetProgram, fmt.Errorf("payload has %d trailing bytes (expected %d, got %d)", uint64(len(payload))-expectedLen, expectedLen, len(payload)) } - ixDataLen := binary.BigEndian.Uint32(payload[offset : offset+4]) - offset += 4 - if offset+int(ixDataLen) > len(payload) { - return nil, nil, 0, targetProgram, fmt.Errorf("payload too short for ix_data") - } - ixData := make([]byte, ixDataLen) - copy(ixData, payload[offset:offset+int(ixDataLen)]) - offset += int(ixDataLen) + ixDataOff := int(ixDataLenOff) + sizeIxDataLen + instrIDOff := ixDataOff + int(ixDataLen) + targetOff := instrIDOff + sizeInstructionID - if offset >= len(payload) { - return nil, nil, 0, targetProgram, fmt.Errorf("payload too short for instruction_id") + instructionID := payload[instrIDOff] + if instructionID != 1 && instructionID != 2 { + return nil, nil, 0, targetProgram, fmt.Errorf("invalid instruction_id %d (expected 1=withdraw or 2=execute)", instructionID) + } + if instructionID == 1 && (accountsCount != 0 || ixDataLen != 0) { + return nil, nil, 0, targetProgram, fmt.Errorf("withdraw payload must have accountsCount=0 and ixDataLen=0, got %d/%d", accountsCount, ixDataLen) } - instructionID := payload[offset] - offset++ - if offset+32 > len(payload) { - return nil, nil, 0, targetProgram, fmt.Errorf("payload too short for target_program") + // --- Parse validated payload --- + + accounts := make([]GatewayAccountMeta, accountsCount) + for i := range accountsCount { + base := sizeAccountsCount + int(i)*sizeAccount + copy(accounts[i].Pubkey[:], payload[base:base+32]) + accounts[i].IsWritable = payload[base+32] == 1 } - copy(targetProgram[:], payload[offset:offset+32]) + + ixData := make([]byte, ixDataLen) + copy(ixData, payload[ixDataOff:instrIDOff]) + + copy(targetProgram[:], payload[targetOff:targetOff+sizeTargetProgram]) return accounts, ixData, instructionID, targetProgram, nil } diff --git a/universalClient/chains/svm/tx_builder_test.go b/universalClient/chains/svm/tx_builder_test.go index a67a0e3d..e58562ae 100644 --- a/universalClient/chains/svm/tx_builder_test.go +++ b/universalClient/chains/svm/tx_builder_test.go @@ -616,69 +616,192 @@ func TestConstructTSSMessage_HashIsKeccak256(t *testing.T) { } func TestDecodePayload(t *testing.T) { - t.Run("decodes valid execute payload with 2 accounts", func(t *testing.T) { - expectedAccounts := []GatewayAccountMeta{ - {Pubkey: makeTxID(0x11), IsWritable: true}, - {Pubkey: makeTxID(0x22), IsWritable: false}, - } - expectedIxData := []byte{0xAA, 0xBB, 0xCC} - expectedTarget := makeTxID(0xDD) - - payload := buildMockPayload(expectedAccounts, expectedIxData, 2, expectedTarget) - accounts, ixData, instructionID, targetProgram, err := decodePayload(payload) - - require.NoError(t, err) - assert.Equal(t, uint8(2), instructionID) - assert.Len(t, accounts, 2) - assert.Equal(t, expectedAccounts[0].Pubkey, accounts[0].Pubkey) - assert.True(t, accounts[0].IsWritable) - assert.Equal(t, expectedAccounts[1].Pubkey, accounts[1].Pubkey) - assert.False(t, accounts[1].IsWritable) - assert.Equal(t, expectedIxData, ixData) - assert.Equal(t, expectedTarget, targetProgram) - }) + // Roundtrip cases: encode with buildMockPayload, decode, assert every field + // round-trips. Each row is a distinct encoding shape we want to support. + roundtripCases := []struct { + name string + accounts []GatewayAccountMeta + ixData []byte + instructionID uint8 + targetProgram [32]byte + }{ + { + name: "execute with 2 accounts (writable + readonly) and ix_data", + accounts: []GatewayAccountMeta{ + {Pubkey: makeTxID(0x11), IsWritable: true}, + {Pubkey: makeTxID(0x22), IsWritable: false}, + }, + ixData: []byte{0xAA, 0xBB, 0xCC}, + instructionID: 2, + targetProgram: makeTxID(0xDD), + }, + { + name: "withdraw with no accounts and no ix_data", + accounts: nil, + ixData: nil, + instructionID: 1, + targetProgram: [32]byte{}, + }, + { + name: "execute with 1 account and empty ix_data", + accounts: []GatewayAccountMeta{{Pubkey: makeTxID(0x33), IsWritable: true}}, + ixData: nil, + instructionID: 2, + targetProgram: makeTxID(0xEE), + }, + } - t.Run("decodes withdraw payload (0 accounts)", func(t *testing.T) { - payload := buildMockWithdrawPayload() - accounts, ixData, instructionID, _, err := decodePayload(payload) - require.NoError(t, err) - assert.Equal(t, uint8(1), instructionID) - assert.Len(t, accounts, 0) - assert.Len(t, ixData, 0) - }) + for _, tc := range roundtripCases { + t.Run(tc.name, func(t *testing.T) { + payload := buildMockPayload(tc.accounts, tc.ixData, tc.instructionID, tc.targetProgram) + accounts, ixData, instructionID, targetProgram, err := decodePayload(payload) + + require.NoError(t, err) + assert.Equal(t, tc.instructionID, instructionID) + assert.Len(t, accounts, len(tc.accounts)) + for i, want := range tc.accounts { + assert.Equal(t, want.Pubkey, accounts[i].Pubkey, "account %d pubkey", i) + assert.Equal(t, want.IsWritable, accounts[i].IsWritable, "account %d writable", i) + } + assert.Equal(t, len(tc.ixData), len(ixData)) + if len(tc.ixData) > 0 { + assert.Equal(t, tc.ixData, ixData) + } + assert.Equal(t, tc.targetProgram, targetProgram) + }) + } - t.Run("decodes payload with empty ix_data", func(t *testing.T) { - accs := []GatewayAccountMeta{{Pubkey: makeTxID(0x33), IsWritable: true}} - expectedTarget := makeTxID(0xEE) - payload := buildMockPayload(accs, nil, 2, expectedTarget) - accounts, ixData, instructionID, targetProgram, err := decodePayload(payload) - require.NoError(t, err) - assert.Equal(t, uint8(2), instructionID) - assert.Len(t, accounts, 1) - assert.Len(t, ixData, 0) - assert.Equal(t, expectedTarget, targetProgram) - }) + // Malformed-payload cases. Each entry constructs a specific bad payload + // and asserts the returned error contains the expected substring. + errCases := []struct { + name string + payload []byte + wantErr string + }{ + { + name: "below 41-byte minimum", + payload: []byte{0, 0}, + wantErr: "payload too short", + }, + { + name: "ix_data_len exceeds remaining bytes", + payload: func() []byte { + // 41-byte payload (exactly minimum). 0 accounts, claims ix_data_len=100 + // but only 33 bytes remain after the two u32 headers. + p := make([]byte, 41) + binary.BigEndian.PutUint32(p[0:4], 0) // accountsCount + binary.BigEndian.PutUint32(p[4:8], 100) // ixDataLen + return p + }(), + wantErr: "payload too short: expected", + }, + { + name: "oversized accountsCount: 0xFFFFFFFF would OOM-panic on make()", + payload: func() []byte { + p := make([]byte, 41) + binary.BigEndian.PutUint32(p[0:4], 0xFFFFFFFF) + return p + }(), + wantErr: "payload too short for 4294967295 accounts", + }, + { + name: "accountsCount exceeds remaining bytes by one (off-by-one boundary)", + payload: func() []byte { + // 4-byte count + 65 bytes is one short of holding 2 accounts (66 bytes). + p := make([]byte, 4+65) + binary.BigEndian.PutUint32(p[0:4], 2) + return p + }(), + wantErr: "payload too short for 2 accounts", + }, + { + name: "oversized ixDataLen: 0xFFFFFFFF must reject before make([]byte)", + payload: func() []byte { + p := make([]byte, 41) + binary.BigEndian.PutUint32(p[0:4], 0) // accountsCount + binary.BigEndian.PutUint32(p[4:8], 0xFFFFFFFF) // ixDataLen + return p + }(), + wantErr: "payload too short: expected", + }, + { + name: "invalid instruction_id (3 is not withdraw/execute)", + payload: func() []byte { + return buildMockPayload(nil, nil, 3, [32]byte{}) + }(), + wantErr: "invalid instruction_id 3", + }, + { + name: "invalid instruction_id (0)", + payload: func() []byte { + return buildMockPayload(nil, nil, 0, [32]byte{}) + }(), + wantErr: "invalid instruction_id 0", + }, + { + name: "withdraw with non-zero accountsCount", + payload: func() []byte { + return buildMockPayload( + []GatewayAccountMeta{{Pubkey: makeTxID(0x11), IsWritable: true}}, + nil, 1, [32]byte{}, + ) + }(), + wantErr: "withdraw payload must have accountsCount=0", + }, + { + name: "withdraw with non-zero ixDataLen", + payload: func() []byte { + return buildMockPayload(nil, []byte{0xAB}, 1, [32]byte{}) + }(), + wantErr: "withdraw payload must have accountsCount=0", + }, + { + name: "trailing bytes after target_program", + payload: func() []byte { + p := buildMockPayload(nil, nil, 1, [32]byte{}) + return append(p, 0x00, 0x01, 0x02) + }(), + wantErr: "trailing bytes", + }, + } - t.Run("rejects too-short payload", func(t *testing.T) { - _, _, _, _, err := decodePayload([]byte{0, 0}) - assert.Error(t, err) - }) + for _, tc := range errCases { + t.Run(tc.name, func(t *testing.T) { + _, _, _, _, err := decodePayload(tc.payload) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + }) + } +} - t.Run("rejects truncated account data", func(t *testing.T) { - // Says 1 account but only provides 10 bytes (need 33) - payload := make([]byte, 4+10) - binary.BigEndian.PutUint32(payload[0:4], 1) - _, _, _, _, err := decodePayload(payload) - assert.Error(t, err) - }) +// FuzzDecodePayload feeds arbitrary byte sequences to decodePayload and asserts +// it never panics. Run locally with: +// +// go test ./chains/svm/ -fuzz=FuzzDecodePayload -fuzztime=30s +// +// The seed corpus mixes valid and known-bad shapes so the fuzzer mutates from +// realistic starting points. +func FuzzDecodePayload(f *testing.F) { + f.Add(buildMockWithdrawPayload()) + f.Add(buildMockPayload( + []GatewayAccountMeta{{Pubkey: makeTxID(0x11), IsWritable: true}}, + []byte{0xAA, 0xBB}, + 2, + makeTxID(0xDD), + )) + f.Add([]byte{}) + f.Add([]byte{0, 0}) + // Adversarial seed: max accountsCount (caught by the OOM guard). + { + p := make([]byte, 41) + binary.BigEndian.PutUint32(p[0:4], 0xFFFFFFFF) + f.Add(p) + } - t.Run("rejects truncated ix_data", func(t *testing.T) { - // 0 accounts, says ix_data len=100 but only 4 bytes remain - payload := make([]byte, 4+4+4) - binary.BigEndian.PutUint32(payload[0:4], 0) // 0 accounts - binary.BigEndian.PutUint32(payload[4:8], 100) // ix_data_len = 100 - _, _, _, _, err := decodePayload(payload) - assert.Error(t, err) + f.Fuzz(func(t *testing.T, payload []byte) { + // Contract: decodePayload returns an error for malformed input; it must + // never panic, OOM, or block indefinitely on any byte sequence. + _, _, _, _, _ = decodePayload(payload) }) }