Move loader enforces account size (#6379)

* Move loader enforces account size

* Fix librapay test

programs/librapay_api/src/librapay_transaction.rs

@@ -75,7 +75,7 @@ pub fn create_accounts(
                 &from.pubkey(),
                 to,
                 lamports,
-                128,
+                200,
                 &solana_move_loader_api::id(),
             )
         })

programs/move_loader_api/src/account_state.rs

@@ -53,7 +53,7 @@ pub enum LibraAccountState {
 }
 impl LibraAccountState {
     pub fn create_unallocated() -> Self {
-        LibraAccountState::Unallocated
+        Self::Unallocated
     }
 
     pub fn create_program(
@@ -66,7 +66,7 @@ impl LibraAccountState {
         let mut extra_deps: Vec<VerifiedModule> = vec![];
         for dep in deps {
             let state: Self = bincode::deserialize(&dep).unwrap();
-            if let LibraAccountState::User(_, write_set) = state {
+            if let Self::User(_, write_set) = state {
                 for (_, write_op) in write_set.iter() {
                     if let WriteOp::Value(raw_bytes) = write_op {
                         extra_deps.push(
@@ -99,13 +99,13 @@ impl LibraAccountState {
                 .expect("Unable to serialize module");
             modules_bytes.push(buf);
         }
-        LibraAccountState::CompiledProgram(
+        Self::CompiledProgram(
             serde_json::to_string(&Program::new(script_bytes, modules_bytes, vec![])).unwrap(),
         )
     }
 
     pub fn create_user(owner: &Pubkey, write_set: WriteSet) -> Self {
-        LibraAccountState::User(*owner, write_set)
+        Self::User(*owner, write_set)
     }
 
     pub fn create_genesis(mint_balance: u64) -> Result<(Self), InstructionError> {
@@ -172,6 +172,6 @@ impl LibraAccountState {
         .freeze()
         .map_err(map_failure_error)?;
 
-        Ok(LibraAccountState::Genesis(write_set))
+        Ok(Self::Genesis(write_set))
     }
 }

programs/move_loader_api/src/error_mappers.rs

@@ -23,7 +23,10 @@ pub fn map_vm_binary_error(err: vm::errors::BinaryError) -> InstructionError {
 #[allow(clippy::needless_pass_by_value)]
 pub fn map_data_error(err: std::boxed::Box<bincode::ErrorKind>) -> InstructionError {
     debug!("Error: Account data: {:?}", err);
-    InstructionError::InvalidAccountData
+    match err.as_ref() {
+        bincode::ErrorKind::SizeLimit => InstructionError::AccountDataTooSmall,
+        _ => InstructionError::InvalidAccountData,
+    }
 }
 #[allow(clippy::needless_pass_by_value)]
 pub fn map_json_error(err: serde_json::error::Error) -> InstructionError {

programs/move_loader_api/src/processor.rs

@@ -95,6 +95,22 @@ impl MoveProcessor {
         locals
     }
 
+    fn serialize_and_enforce_length(
+        state: &LibraAccountState,
+        data: &mut Vec<u8>,
+    ) -> Result<(), InstructionError> {
+        let original_len = data.len() as u64;
+        let mut writer = std::io::Cursor::new(data);
+        bincode::config()
+            .limit(original_len)
+            .serialize_into(&mut writer, &state)
+            .map_err(map_data_error)?;
+        if writer.position() < original_len {
+            writer.set_position(original_len);
+        }
+        Ok(())
+    }
+
     fn serialize_verified_program(
         script: &VerifiedScript,
         modules: &[VerifiedModule],
@@ -172,7 +188,7 @@ impl MoveProcessor {
 
     fn execute(
         sender_address: AccountAddress,
-        function_name: String,
+        function_name: &str,
         args: Vec<TransactionArgument>,
         script: VerifiedScript,
         modules: Vec<VerifiedModule>,
@@ -247,20 +263,20 @@ impl MoveProcessor {
         let write_set = write_sets
             .remove(&AccountAddress::default())
             .ok_or_else(Self::missing_account)?;
-        keyed_accounts[GENESIS_INDEX].account.data.clear();
+        Self::serialize_and_enforce_length(
-        let writer = std::io::BufWriter::new(&mut keyed_accounts[GENESIS_INDEX].account.data);
+            &LibraAccountState::Genesis(write_set),
-        bincode::serialize_into(writer, &LibraAccountState::Genesis(write_set))
+            &mut keyed_accounts[GENESIS_INDEX].account.data,
-            .map_err(map_data_error)?;
+        )?;
 
         // Now do the rest of the accounts
         for keyed_account in keyed_accounts[GENESIS_INDEX + 1..].iter_mut() {
             let write_set = write_sets
                 .remove(&pubkey_to_address(keyed_account.unsigned_key()))
                 .ok_or_else(Self::missing_account)?;
-            keyed_account.account.data.clear();
+            Self::serialize_and_enforce_length(
-            let writer = std::io::BufWriter::new(&mut keyed_account.account.data);
+                &LibraAccountState::User(genesis_key, write_set),
-            bincode::serialize_into(writer, &LibraAccountState::User(genesis_key, write_set))
+                &mut keyed_account.account.data,
-                .map_err(map_data_error)?;
+            )?;
         }
         if !write_sets.is_empty() {
             debug!("Error: Missing keyed accounts");
@@ -345,12 +361,10 @@ impl MoveProcessor {
                 match bincode::deserialize(&keyed_accounts[0].account.data)
                     .map_err(map_data_error)?
                 {
-                    LibraAccountState::Unallocated => {
+                    LibraAccountState::Unallocated => Self::serialize_and_enforce_length(
-                        keyed_accounts[0].account.data.clear();
+                        &LibraAccountState::create_genesis(amount)?,
-                        let writer = std::io::BufWriter::new(&mut keyed_accounts[0].account.data);
+                        &mut keyed_accounts[0].account.data,
-                        bincode::serialize_into(writer, &LibraAccountState::create_genesis(amount)?)
+                    ),
-                            .map_err(map_data_error)
-                    }
                     _ => {
                         debug!("Error: Must provide an unallocated account");
                         Err(InstructionError::InvalidArgument)
@@ -385,7 +399,7 @@ impl MoveProcessor {
 
                 let output = Self::execute(
                     sender_address,
-                    function_name,
+                    &function_name,
                     args,
                     verified_script,
                     verified_modules,
@@ -410,6 +424,34 @@ mod tests {
     use solana_sdk::rent_calculator::RentCalculator;
     use solana_sdk::sysvar::rent;
 
+    const BIG_ENOUGH: usize = 6_000;
+
+    #[test]
+    fn test_account_size() {
+        let mut data =
+            vec![0_u8; bincode::serialized_size(&LibraAccountState::Unallocated).unwrap() as usize];
+        let len = data.len();
+        assert_eq!(
+            MoveProcessor::serialize_and_enforce_length(&LibraAccountState::Unallocated, &mut data),
+            Ok(())
+        );
+        assert_eq!(len, data.len());
+
+        data.resize(6000, 0);
+        let len = data.len();
+        assert_eq!(
+            MoveProcessor::serialize_and_enforce_length(&LibraAccountState::Unallocated, &mut data),
+            Ok(())
+        );
+        assert_eq!(len, data.len());
+
+        data.resize(1, 0);
+        assert_eq!(
+            MoveProcessor::serialize_and_enforce_length(&LibraAccountState::Unallocated, &mut data),
+            Err(InstructionError::AccountDataTooSmall)
+        );
+    }
+
     #[test]
     fn test_finalize() {
         solana_logger::setup();
@@ -439,6 +481,7 @@ mod tests {
             false,
             &mut unallocated.account,
         )];
+        keyed_accounts[0].account.data.resize(BIG_ENOUGH, 0);
         MoveProcessor::do_invoke_main(
             &mut keyed_accounts,
             &bincode::serialize(&InvokeCommand::CreateGenesis(amount)).unwrap(),
@@ -584,6 +627,7 @@ mod tests {
             KeyedAccount::new(&program.key, true, &mut program.account),
             KeyedAccount::new(&rent_id, false, &mut rent_account),
         ];
+        keyed_accounts[0].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_finalize(&mut keyed_accounts).unwrap();
 
@@ -593,6 +637,8 @@ mod tests {
             KeyedAccount::new(&sender.key, false, &mut sender.account),
             KeyedAccount::new(&payee.key, false, &mut payee.account),
         ];
+        keyed_accounts[2].account.data.resize(BIG_ENOUGH, 0);
+        keyed_accounts[3].account.data.resize(BIG_ENOUGH, 0);
 
         let amount = 2;
         MoveProcessor::do_invoke_main(
@@ -656,6 +702,7 @@ mod tests {
             KeyedAccount::new(&program.key, true, &mut program.account),
             KeyedAccount::new(&rent_id, false, &mut rent_account),
         ];
+        keyed_accounts[0].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_finalize(&mut keyed_accounts).unwrap();
 
@@ -664,6 +711,7 @@ mod tests {
             KeyedAccount::new(&genesis.key, false, &mut genesis.account),
             KeyedAccount::new(&payee.key, false, &mut payee.account),
         ];
+        keyed_accounts[2].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_invoke_main(
             &mut keyed_accounts,
@@ -700,6 +748,7 @@ mod tests {
             KeyedAccount::new(&program.key, true, &mut program.account),
             KeyedAccount::new(&rent_id, false, &mut rent_account),
         ];
+        keyed_accounts[0].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_finalize(&mut keyed_accounts).unwrap();
 
@@ -708,6 +757,7 @@ mod tests {
             KeyedAccount::new(&genesis.key, false, &mut genesis.account),
             KeyedAccount::new(&module.key, false, &mut module.account),
         ];
+        keyed_accounts[2].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_invoke_main(
             &mut keyed_accounts,
@@ -784,6 +834,7 @@ mod tests {
             KeyedAccount::new(&program.key, true, &mut program.account),
             KeyedAccount::new(&rent_id, false, &mut rent_account),
         ];
+        keyed_accounts[0].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_finalize(&mut keyed_accounts).unwrap();
 
@@ -792,6 +843,7 @@ mod tests {
             KeyedAccount::new(&genesis.key, false, &mut genesis.account),
             KeyedAccount::new(&payee.key, false, &mut payee.account),
         ];
+        keyed_accounts[2].account.data.resize(BIG_ENOUGH, 0);
 
         MoveProcessor::do_invoke_main(
             &mut keyed_accounts,