Return InvalidInstructionData for malformed token metadata instructions

clients/rust-legacy/tests/token_metadata_initialize.rs

@@ -9,7 +9,9 @@ use {
     },
     spl_token_2022_interface::{error::TokenError, extension::BaseStateWithExtensions},
     spl_token_client::token::{ExtensionInitializationParams, TokenError as TokenClientError},
-    spl_token_metadata_interface::{error::TokenMetadataError, state::TokenMetadata},
+    spl_token_metadata_interface::{
+        error::TokenMetadataError, instruction::Initialize, state::TokenMetadata,
+    },
     std::{convert::TryInto, sync::Arc},
 };
 
@@ -282,3 +284,53 @@ async fn fail_without_signature() {
         )))
     );
 }
+
+#[tokio::test]
+async fn fail_initialize_with_forged_name_length_returns_invalid_instruction_data() {
+    let authority = Pubkey::new_unique();
+    let mint_keypair = Keypair::new();
+    let mut test_context = setup(mint_keypair, &authority).await;
+
+    let token_context = test_context.token_context.take().unwrap();
+
+    let name = "Name".to_string();
+    let symbol = "SYM".to_string();
+    let uri = "uri".to_string();
+    let payload = borsh::to_vec(&Initialize {
+        name: name.clone(),
+        symbol: symbol.clone(),
+        uri: uri.clone(),
+    })
+    .unwrap();
+    let mut instruction = spl_token_metadata_interface::instruction::initialize(
+        &spl_token_2022_interface::id(),
+        token_context.token.get_address(),
+        &Pubkey::new_unique(),
+        token_context.token.get_address(),
+        &token_context.mint_authority.pubkey(),
+        name,
+        symbol,
+        uri,
+    );
+
+    let payload_offset = instruction.data.len().checked_sub(payload.len()).unwrap();
+    let length_prefix_len = std::mem::size_of::<u32>();
+    instruction.data[payload_offset..payload_offset + length_prefix_len]
+        .copy_from_slice(&u32::MAX.to_le_bytes());
+
+    let error = token_context
+        .token
+        .process_ixs(&[instruction], &[&token_context.mint_authority])
+        .await
+        .unwrap_err();
+
+    // Once the metadata decoder is hardened, malformed recognized payloads
+    // should reject as InvalidInstructionData instead of aborting or falling
+    // through to another error shape.
+    assert_eq!(
+        error,
+        TokenClientError::Client(Box::new(TransportError::TransactionError(
+            TransactionError::InstructionError(0, InstructionError::InvalidInstructionData)
+        )))
+    );
+}

clients/rust-legacy/tests/token_metadata_remove_key.rs

@@ -14,7 +14,7 @@ use {
     spl_token_client::token::{ExtensionInitializationParams, TokenError as TokenClientError},
     spl_token_metadata_interface::{
         error::TokenMetadataError,
-        instruction::remove_key,
+        instruction::{remove_key, RemoveKey},
         state::{Field, TokenMetadata},
     },
     std::{convert::TryInto, sync::Arc},
@@ -250,3 +250,65 @@ async fn fail_authority_checks() {
         TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature,)
     );
 }
+
+#[tokio::test]
+async fn fail_remove_key_with_forged_key_length_returns_invalid_instruction_data() {
+    let authority = Keypair::new();
+    let mint_keypair = Keypair::new();
+    let mut test_context = setup(mint_keypair, &authority.pubkey()).await;
+    let payer_pubkey = test_context.context.lock().await.payer.pubkey();
+    let token_context = test_context.token_context.take().unwrap();
+
+    let update_authority = Keypair::new();
+    token_context
+        .token
+        .token_metadata_initialize_with_rent_transfer(
+            &payer_pubkey,
+            &update_authority.pubkey(),
+            &token_context.mint_authority.pubkey(),
+            "MySuperCoolToken".to_string(),
+            "MINE".to_string(),
+            "my.super.cool.token".to_string(),
+            &[&token_context.mint_authority],
+        )
+        .await
+        .unwrap();
+
+    let key = "new_name".to_string();
+    let idempotent = true;
+    let idempotent_prefix = borsh::to_vec(&idempotent).unwrap();
+    let payload = borsh::to_vec(&RemoveKey {
+        key: key.clone(),
+        idempotent,
+    })
+    .unwrap();
+    let mut instruction = remove_key(
+        &spl_token_2022_interface::id(),
+        token_context.token.get_address(),
+        &update_authority.pubkey(),
+        key,
+        idempotent,
+    );
+
+    let payload_offset = instruction.data.len().checked_sub(payload.len()).unwrap();
+    let key_length_offset = payload_offset + idempotent_prefix.len();
+    let length_prefix_len = std::mem::size_of::<u32>();
+    instruction.data[key_length_offset..key_length_offset + length_prefix_len]
+        .copy_from_slice(&u32::MAX.to_le_bytes());
+
+    let error = token_context
+        .token
+        .process_ixs(&[instruction], &[&update_authority])
+        .await
+        .unwrap_err();
+
+    // Once the metadata decoder is hardened, malformed recognized payloads
+    // should reject as InvalidInstructionData instead of aborting or falling
+    // through to another error shape.
+    assert_eq!(
+        error,
+        TokenClientError::Client(Box::new(TransportError::TransactionError(
+            TransactionError::InstructionError(0, InstructionError::InvalidInstructionData)
+        )))
+    );
+}

clients/rust-legacy/tests/token_metadata_update_field.rs

@@ -11,7 +11,7 @@ use {
     spl_token_client::token::{ExtensionInitializationParams, TokenError as TokenClientError},
     spl_token_metadata_interface::{
         error::TokenMetadataError,
-        instruction::update_field,
+        instruction::{update_field, UpdateField},
         state::{Field, TokenMetadata},
     },
     std::{convert::TryInto, sync::Arc},
@@ -198,3 +198,65 @@ async fn fail_authority_checks() {
         )))
     );
 }
+
+#[tokio::test]
+async fn fail_update_field_with_forged_value_length_returns_invalid_instruction_data() {
+    let authority = Keypair::new();
+    let mint_keypair = Keypair::new();
+    let mut test_context = setup(mint_keypair, &authority.pubkey()).await;
+    let payer_pubkey = test_context.context.lock().await.payer.pubkey();
+    let token_context = test_context.token_context.take().unwrap();
+
+    let update_authority = Keypair::new();
+    token_context
+        .token
+        .token_metadata_initialize_with_rent_transfer(
+            &payer_pubkey,
+            &update_authority.pubkey(),
+            &token_context.mint_authority.pubkey(),
+            "MySuperCoolToken".to_string(),
+            "MINE".to_string(),
+            "my.super.cool.token".to_string(),
+            &[&token_context.mint_authority],
+        )
+        .await
+        .unwrap();
+
+    let field = Field::Name;
+    let value = "new_name".to_string();
+    let field_prefix = borsh::to_vec(&field).unwrap();
+    let payload = borsh::to_vec(&UpdateField {
+        field: field.clone(),
+        value: value.clone(),
+    })
+    .unwrap();
+    let mut instruction = update_field(
+        &spl_token_2022_interface::id(),
+        token_context.token.get_address(),
+        &update_authority.pubkey(),
+        field,
+        value,
+    );
+
+    let payload_offset = instruction.data.len().checked_sub(payload.len()).unwrap();
+    let value_length_offset = payload_offset + field_prefix.len();
+    let length_prefix_len = std::mem::size_of::<u32>();
+    instruction.data[value_length_offset..value_length_offset + length_prefix_len]
+        .copy_from_slice(&u32::MAX.to_le_bytes());
+
+    let error = token_context
+        .token
+        .process_ixs(&[instruction], &[&update_authority])
+        .await
+        .unwrap_err();
+
+    // Once the metadata decoder is hardened, malformed recognized payloads
+    // should reject as InvalidInstructionData instead of aborting or falling
+    // through to another error shape.
+    assert_eq!(
+        error,
+        TokenClientError::Client(Box::new(TransportError::TransactionError(
+            TransactionError::InstructionError(0, InstructionError::InvalidInstructionData)
+        )))
+    );
+}

program/src/processor.rs

@@ -73,6 +73,15 @@ use {
     std::convert::{TryFrom, TryInto},
 };
 
+const TOKEN_METADATA_DISCRIMINATOR_LENGTH: usize = 8;
+const U32_BYTES: usize = 4;
+const TOKEN_METADATA_INITIALIZE_DISCRIMINATOR: [u8; TOKEN_METADATA_DISCRIMINATOR_LENGTH] =
+    [0xd2, 0xe1, 0x1e, 0xa2, 0x58, 0xb8, 0x4d, 0x8d];
+const TOKEN_METADATA_UPDATE_FIELD_DISCRIMINATOR: [u8; TOKEN_METADATA_DISCRIMINATOR_LENGTH] =
+    [0xdd, 0xe9, 0x31, 0x2d, 0xb5, 0xca, 0xdc, 0xc8];
+const TOKEN_METADATA_REMOVE_KEY_DISCRIMINATOR: [u8; TOKEN_METADATA_DISCRIMINATOR_LENGTH] =
+    [0xea, 0x12, 0x20, 0x38, 0x59, 0x8d, 0x25, 0xb5];
+
 pub(crate) enum TransferInstruction {
     Unchecked,
     Checked { decimals: u8 },
@@ -96,6 +105,98 @@ pub(crate) enum BurnInstructionVariant {
 /// Program state handler.
 pub struct Processor {}
 impl Processor {
+    fn read_u8(input: &[u8], cursor: &mut usize) -> Result<u8, ProgramError> {
+        let value = *input
+            .get(*cursor)
+            .ok_or(ProgramError::InvalidInstructionData)?;
+        *cursor = cursor
+            .checked_add(1)
+            .ok_or(ProgramError::InvalidInstructionData)?;
+        Ok(value)
+    }
+
+    fn read_u32(input: &[u8], cursor: &mut usize) -> Result<u32, ProgramError> {
+        let end = cursor
+            .checked_add(U32_BYTES)
+            .ok_or(ProgramError::InvalidInstructionData)?;
+        let bytes = input
+            .get(*cursor..end)
+            .and_then(|slice| slice.try_into().ok())
+            .ok_or(ProgramError::InvalidInstructionData)?;
+        *cursor = end;
+        Ok(u32::from_le_bytes(bytes))
+    }
+
+    fn skip_borsh_string(input: &[u8], cursor: &mut usize) -> ProgramResult {
+        let length = usize::try_from(Self::read_u32(input, cursor)?)
+            .map_err(|_| ProgramError::InvalidInstructionData)?;
+        let end = cursor
+            .checked_add(length)
+            .ok_or(ProgramError::InvalidInstructionData)?;
+        input
+            .get(*cursor..end)
+            .ok_or(ProgramError::InvalidInstructionData)?;
+        *cursor = end;
+        Ok(())
+    }
+
+    fn validate_no_remaining_bytes(input: &[u8], cursor: usize) -> ProgramResult {
+        if cursor == input.len() {
+            Ok(())
+        } else {
+            Err(ProgramError::InvalidInstructionData)
+        }
+    }
+
+    fn validate_token_metadata_initialize_payload(input: &[u8]) -> ProgramResult {
+        let mut cursor = 0;
+        Self::skip_borsh_string(input, &mut cursor)?;
+        Self::skip_borsh_string(input, &mut cursor)?;
+        Self::skip_borsh_string(input, &mut cursor)?;
+        Self::validate_no_remaining_bytes(input, cursor)
+    }
+
+    fn validate_token_metadata_update_field_payload(input: &[u8]) -> ProgramResult {
+        let mut cursor = 0;
+        match Self::read_u8(input, &mut cursor)? {
+            0..=2 => {}
+            3 => Self::skip_borsh_string(input, &mut cursor)?,
+            _ => return Err(ProgramError::InvalidInstructionData),
+        }
+        Self::skip_borsh_string(input, &mut cursor)?;
+        Self::validate_no_remaining_bytes(input, cursor)
+    }
+
+    fn validate_token_metadata_remove_key_payload(input: &[u8]) -> ProgramResult {
+        let mut cursor = 0;
+        match Self::read_u8(input, &mut cursor)? {
+            0 | 1 => {}
+            _ => return Err(ProgramError::InvalidInstructionData),
+        }
+        Self::skip_borsh_string(input, &mut cursor)?;
+        Self::validate_no_remaining_bytes(input, cursor)
+    }
+
+    fn validate_affected_token_metadata_instruction(input: &[u8]) -> Result<bool, ProgramError> {
+        if input.len() < TOKEN_METADATA_DISCRIMINATOR_LENGTH {
+            return Ok(false);
+        }
+
+        let (discriminator, rest) = input.split_at(TOKEN_METADATA_DISCRIMINATOR_LENGTH);
+        if discriminator == TOKEN_METADATA_INITIALIZE_DISCRIMINATOR.as_slice() {
+            Self::validate_token_metadata_initialize_payload(rest)?;
+            Ok(true)
+        } else if discriminator == TOKEN_METADATA_UPDATE_FIELD_DISCRIMINATOR.as_slice() {
+            Self::validate_token_metadata_update_field_payload(rest)?;
+            Ok(true)
+        } else if discriminator == TOKEN_METADATA_REMOVE_KEY_DISCRIMINATOR.as_slice() {
+            Self::validate_token_metadata_remove_key_payload(rest)?;
+            Ok(true)
+        } else {
+            Ok(false)
+        }
+    }
+
     fn _process_initialize_mint(
         accounts: &[AccountInfo],
         decimals: u8,
@@ -2113,6 +2214,10 @@ impl Processor {
                     Self::process_unwrap_lamports(program_id, accounts, amount)
                 }
             }
+        } else if Self::validate_affected_token_metadata_instruction(input)? {
+            let instruction = TokenMetadataInstruction::unpack(input)
+                .map_err(|_| ProgramError::InvalidInstructionData)?;
+            token_metadata::processor::process_instruction(program_id, accounts, instruction)
         } else if let Ok(instruction) = TokenMetadataInstruction::unpack(input) {
             token_metadata::processor::process_instruction(program_id, accounts, instruction)
         } else if let Ok(instruction) = TokenGroupInstruction::unpack(input) {