contracts/ccip/ccip: validate account address lengths

contracts/ccip/ccip/sources/client.move

@@ -1,11 +1,13 @@
 /// This module defines messages for end users to interact with Aptos CCIP.
 module ccip::client {
     use std::bcs;
+    use std::error;
 
     const GENERIC_EXTRA_ARGS_V2_TAG: vector<u8> = x"181dcf10";
     const SVM_EXTRA_ARGS_V1_TAG: vector<u8> = x"1f3b3aba";
 
     const E_INVALID_SVM_TOKEN_RECEIVER_LENGTH: u64 = 1;
+    const E_INVALID_SVM_ACCOUNT_LENGTH: u64 = 2;
 
     #[view]
     public fun generic_extra_args_v2_tag(): vector<u8> {
@@ -41,19 +43,35 @@ module ccip::client {
         extra_args.append(bcs::to_bytes(&compute_units));
         extra_args.append(bcs::to_bytes(&account_is_writable_bitmap));
         extra_args.append(bcs::to_bytes(&allow_out_of_order_execution));
-        if (token_receiver.length() < 32) {
-            token_receiver.reverse();
-            while (token_receiver.length() < 32) {
-                token_receiver.push_back(0);
-            };
-            token_receiver.reverse();
-        };
-        assert!(token_receiver.length() == 32, E_INVALID_SVM_TOKEN_RECEIVER_LENGTH);
+        pad_svm_address(&mut token_receiver);
+        assert!(
+            token_receiver.length() == 32,
+            error::invalid_argument(E_INVALID_SVM_TOKEN_RECEIVER_LENGTH)
+        );
         extra_args.append(bcs::to_bytes(&token_receiver));
+        let accounts_len = accounts.length();
+        for (i in 0..accounts_len) {
+            let account = accounts.borrow_mut(i);
+            pad_svm_address(account);
+            assert!(
+                account.length() == 32,
+                error::invalid_argument(E_INVALID_SVM_ACCOUNT_LENGTH)
+            );
+        };
         extra_args.append(bcs::to_bytes(&accounts));
         extra_args
     }
 
+    inline fun pad_svm_address(svm_address: &mut vector<u8>) {
+        if (svm_address.length() < 32) {
+            svm_address.reverse();
+            while (svm_address.length() < 32) {
+                svm_address.push_back(0);
+            };
+            svm_address.reverse();
+        }
+    }
+
     struct Any2AptosMessage has store, drop, copy {
         message_id: vector<u8>,
         source_chain_selector: u64,
@@ -125,4 +143,11 @@ module ccip::client {
     public fun get_amount(input: &Any2AptosTokenAmount): u64 {
         input.amount
     }
+
+    // ======================== Test functions ========================
+
+    #[test_only]
+    public fun test_pad_svm_address(svm_address: &mut vector<u8>) {
+        pad_svm_address(svm_address);
+    }
 }

contracts/ccip/ccip/sources/fee_quoter.move

@@ -246,6 +246,7 @@ module ccip::fee_quoter {
     const E_INVALID_FEE_RANGE: u64 = 34;
     const E_INVALID_DEST_BYTES_OVERHEAD: u64 = 35;
     const E_INVALID_SVM_RECEIVER_LENGTH: u64 = 36;
+    const E_INVALID_SVM_ACCOUNT_LENGTH: u64 = 37;
 
     #[view]
     public fun type_and_version(): String {
@@ -901,6 +902,13 @@ module ccip::fee_quoter {
             ) * SVM_ACCOUNT_BYTE_SIZE);
         };
 
+        for (i in 0..accounts_length) {
+            assert!(
+                accounts[i].length() == 32,
+                error::invalid_argument(E_INVALID_SVM_ACCOUNT_LENGTH)
+            );
+        };
+
         if (tokens_len > 0) {
             assert!(
                 token_receiver.length() == 32

contracts/ccip/ccip/tests/client_test.move

@@ -156,17 +156,193 @@ module ccip::client_test {
     }
 
     #[test]
-    #[
-        expected_failure(
-            abort_code = client::E_INVALID_SVM_TOKEN_RECEIVER_LENGTH,
-            location = ccip::client
-        )
-    ]
+    #[expected_failure(abort_code = 65537, location = ccip::client)]
+    // E_INVALID_SVM_TOKEN_RECEIVER_LENGTH
     fun test_svm_args_rejects_long_token_receiver() {
         // Test that token receivers longer than 32 bytes are rejected
         let long_receiver =
             x"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"; // 50 bytes
         // E_INVALID_SVM_TOKEN_RECEIVER_LENGTH
         client::encode_svm_extra_args_v1(100, 0, false, long_receiver, vector[]);
     }
+
+    #[test]
+    fun test_pad_svm_address_empty() {
+        let addr = vector[];
+        client::test_pad_svm_address(&mut addr);
+        assert!(addr.length() == 32, 0);
+        // Verify all bytes are zeros
+        for (i in 0..32) {
+            assert!(*addr.borrow(i) == 0, 1);
+        };
+    }
+
+    #[test]
+    fun test_pad_svm_address_single_byte() {
+        let addr = vector[0x42];
+        client::test_pad_svm_address(&mut addr);
+        assert!(addr.length() == 32, 0);
+        // First 31 bytes should be zero, last byte should be 0x42
+        for (i in 0..31) {
+            assert!(*addr.borrow(i) == 0, 1);
+        };
+        assert!(*addr.borrow(31) == 0x42, 2);
+    }
+
+    #[test]
+    fun test_pad_svm_address_partial() {
+        let addr = vector[0x01, 0x02, 0x03, 0x04];
+        client::test_pad_svm_address(&mut addr);
+        assert!(addr.length() == 32, 0);
+        // First 28 bytes should be zero, last 4 bytes should be the original data
+        for (i in 0..28) {
+            assert!(*addr.borrow(i) == 0, 1);
+        };
+        assert!(*addr.borrow(28) == 0x01, 2);
+        assert!(*addr.borrow(29) == 0x02, 3);
+        assert!(*addr.borrow(30) == 0x03, 4);
+        assert!(*addr.borrow(31) == 0x04, 5);
+    }
+
+    #[test]
+    fun test_pad_svm_address_exact_32_bytes() {
+        let addr = vector[];
+        for (i in 0..32) {
+            addr.push_back((i as u8));
+        };
+        let original_addr = addr;
+        client::test_pad_svm_address(&mut addr);
+        assert!(addr.length() == 32, 0);
+        // Should remain unchanged since it's already 32 bytes
+        assert!(addr == original_addr, 1);
+    }
+
+    #[test]
+    fun test_pad_svm_address_31_bytes() {
+        let addr = vector[];
+        for (i in 0..31) {
+            addr.push_back((i as u8));
+        };
+        client::test_pad_svm_address(&mut addr);
+        assert!(addr.length() == 32, 0);
+        // First byte should be 0 (padding), rest should be the original data
+        assert!(*addr.borrow(0) == 0, 1);
+        for (i in 1..32) {
+            assert!(*addr.borrow(i) == ((i - 1) as u8), 2);
+        };
+    }
+
+    #[test]
+    fun test_encode_svm_extra_args_v1_basic() {
+        let token_receiver = vector[0x01, 0x02, 0x03];
+        let accounts = vector[vector[0x04, 0x05], vector[0x06, 0x07, 0x08]];
+
+        let result =
+            client::encode_svm_extra_args_v1(
+                1000u32, 0u64, true, token_receiver, accounts
+            );
+
+        // Verify the result starts with the correct tag
+        let tag_len = client::svm_extra_args_v1_tag().length();
+        for (i in 0..tag_len) {
+            assert!(
+                *result.borrow(i) == *client::svm_extra_args_v1_tag().borrow(i),
+                0
+            );
+        };
+
+        // Result should be non-empty and contain the tag
+        assert!(result.length() > tag_len, 1);
+    }
+
+    #[test]
+    fun test_encode_svm_extra_args_v1_empty_accounts() {
+        let token_receiver = vector[0xFF];
+        let accounts = vector[];
+
+        let result =
+            client::encode_svm_extra_args_v1(
+                500u32, 0u64, false, token_receiver, accounts
+            );
+
+        // Should not fail and should contain the tag
+        let tag_len = client::svm_extra_args_v1_tag().length();
+        assert!(result.length() > tag_len, 0);
+    }
+
+    #[test]
+    fun test_encode_svm_extra_args_v1_32_byte_addresses() {
+        let token_receiver = vector[];
+        let account1 = vector[];
+        let account2 = vector[];
+
+        // Create exactly 32-byte addresses
+        for (i in 0..32) {
+            token_receiver.push_back((i as u8));
+            account1.push_back(((i + 100) as u8));
+            account2.push_back(((i + 200) as u8));
+        };
+
+        let accounts = vector[account1, account2];
+
+        let result =
+            client::encode_svm_extra_args_v1(
+                2000u32,
+                0xFFFFFFFFFFFFFFFFu64,
+                true,
+                token_receiver,
+                accounts
+            );
+
+        // Should succeed without padding since addresses are already 32 bytes
+        let tag_len = client::svm_extra_args_v1_tag().length();
+        assert!(result.length() > tag_len, 0);
+    }
+
+    #[test]
+    fun test_encode_svm_extra_args_v1_mixed_address_lengths() {
+        let token_receiver = vector[0x11]; // 1 byte
+        let accounts = vector[
+            vector[0x22, 0x33], // 2 bytes
+            vector[], // 0 bytes (empty)
+            vector[0x44, 0x55, 0x66, 0x77, 0x88] // 5 bytes
+        ];
+
+        let result =
+            client::encode_svm_extra_args_v1(
+                750u32, 0u64, false, token_receiver, accounts
+            );
+
+        // All addresses should be padded to 32 bytes internally
+        let tag_len = client::svm_extra_args_v1_tag().length();
+        assert!(result.length() > tag_len, 0);
+    }
+
+    #[test]
+    #[expected_failure(abort_code = 65537)]
+    // E_INVALID_SVM_TOKEN_RECEIVER_LENGTH
+    fun test_encode_svm_extra_args_v1_invalid_token_receiver_length() {
+        // This test should fail because we're creating a token_receiver that's longer than 32 bytes
+        let token_receiver = vector[];
+        for (i in 0..33) { // 33 bytes - too long
+            token_receiver.push_back((i as u8));
+        };
+        let accounts = vector[];
+        client::encode_svm_extra_args_v1(1000u32, 0u64, true, token_receiver, accounts);
+    }
+
+    #[test]
+    #[expected_failure(abort_code = 65538)]
+    // E_INVALID_SVM_ACCOUNT_LENGTH
+    fun test_encode_svm_extra_args_v1_invalid_account_length() {
+        // This test should fail because we're creating an account that's longer than 32 bytes
+        let token_receiver = vector[0x01];
+        let long_account = vector[];
+        for (i in 0..33) { // 33 bytes - too long
+            long_account.push_back((i as u8));
+        };
+        let accounts = vector[long_account];
+
+        client::encode_svm_extra_args_v1(1000u32, 0u64, true, token_receiver, accounts);
+    }
 }

contracts/ccip/ccip/tests/fee_quoter/fee_quoter_bcs.move

@@ -61,7 +61,13 @@ module ccip::fee_quoter_bcs {
         assert!(decoded_bitmap == bitmap);
         assert!(decoded_allow_ooo_svm == allow_ooo_svm);
         assert!(decoded_token_receiver == token_receiver);
-        assert!(decoded_accounts == accounts);
+
+        // SVM addresses are padded to 32 bytes during encoding, so decoded accounts will be 32 bytes
+        let expected_accounts = vector[
+            x"0000000000000000000000000000000000000000000000000000000102030405",
+            x"0000000000000000000000000000000000000000000000000000000504030201"
+        ];
+        assert!(decoded_accounts == expected_accounts);
     }
 
     #[test]
@@ -139,17 +145,35 @@ module ccip::fee_quoter_bcs {
                 100u32, 200u64, false, token_receiver, accounts
             );
 
-        // Expected size breakdown:
-        // 4 bytes (SVM_EXTRA_ARGS_V1_TAG)
-        // 4 bytes (u32 compute_units = 100)
-        // 8 bytes (u64 bitmap = 200)
-        // 1 byte (bool allow_ooo = false)
-        // 33 bytes (BCS-encoded token_receiver: 1 byte length + 32 bytes data)
-        // 9 bytes (BCS-encoded accounts: 1 byte outer length + 4 bytes first inner + 4 bytes second inner)
-        //   where accounts = vector[vector[1,2,3], vector[4,5,6]]
-        //   = 02 030102030 0304050 6 (in hex)
-        // Total: 4 + 4 + 8 + 1 + 33 + 9 = 59 bytes
-        assert!(svm_encoded.length() == 59); // Exact expected size
+        // Expected size breakdown for BCS encoding:
+        //
+        // 1. Fixed-size fields:
+        //    - 4 bytes: SVM_EXTRA_ARGS_V1_TAG
+        //    - 4 bytes: u32 compute_units = 100
+        //    - 8 bytes: u64 bitmap = 200
+        //    - 1 byte:  bool allow_ooo = false
+        //    Subtotal: 17 bytes
+        //
+        // 2. BCS-encoded token_receiver (vector<u8>):
+        //    - 1 byte: vector length (32)
+        //    - 32 bytes: actual data (already 32 bytes, no padding needed)
+        //    Subtotal: 33 bytes
+        //
+        // 3. BCS-encoded accounts (vector<vector<u8>>):
+        //    Original accounts: [vector[1,2,3], vector[4,5,6]] (2 accounts of 3 bytes each)
+        //    After SVM padding: [32-byte account1, 32-byte account2]
+        //
+        //    BCS encoding structure:
+        //    - 1 byte: outer vector length (2 accounts)
+        //    - For each account (BCS-encoded vector<u8>):
+        //      - 1 byte: inner vector length (32)
+        //      - 32 bytes: padded account data
+        //
+        //    Calculation: 1 + (1 + 32) + (1 + 32) = 1 + 33 + 33 = 67 bytes
+        //    Subtotal: 67 bytes
+        //
+        // Total: 17 + 33 + 67 = 117 bytes
+        assert!(svm_encoded.length() == 117); // Exact expected size with 32-byte SVM addresses
     }
 
     #[test]
@@ -297,7 +321,14 @@ module ccip::fee_quoter_bcs {
         assert!(decoded_bitmap2 == bitmap2, 6);
         assert!(decoded_ooo2 == allow_ooo2, 7);
         assert!(decoded_receiver2 == token_receiver2, 8);
-        assert!(decoded_accounts2 == accounts2, 9);
+
+        // SVM addresses are padded to 32 bytes during encoding
+        let expected_accounts2 = vector[
+            x"0000000000000000000000000000000000000000000000000000000000010203",
+            x"0000000000000000000000000000000000000000000000000000000000040506",
+            x"0000000000000000000000000000000000000000000000000000000000070809"
+        ];
+        assert!(decoded_accounts2 == expected_accounts2, 9);
 
         // Test case 3: Zero receiver case
         let compute_units3 = 1000000u32;