Undefined behavior due to MaybeUninit initialization invariant
Description
The token program makes use of MaybeUninit in almost all of its functions. In the type's documentation, we find ways that incorrect usage can lead to undefined behavior in the form of an initialization invariant↗. This happens when uninitialized memory is read from without first being written to.
In the invoke_signed functions of initialize_mint, initialize_mint2, and set_authority, this is the case.
pub fn invoke_signed(&self, signers: &[Signer]) -> ProgramResult {
// Account metadata
let account_metas: [AccountMeta; 2] = [
AccountMeta::writable(self.mint.key()),
AccountMeta::readonly(self.rent_sysvar.key()),
];
// Instruction data layout:
// - [0]: instruction discriminator (1 byte, u8)
// - [1]: decimals (1 byte, u8)
// - [2..34]: mint_authority (32 bytes, Pubkey)
// - [34]: freeze_authority presence flag (1 byte, u8)
// - [35..67]: freeze_authority (optional, 32 bytes, Pubkey)
let mut instruction_data = [UNINIT_BYTE; 67]; <-- MaybeUninit uninitialized bytes
// Set discriminator as u8 at offset [0]
write_bytes(&mut instruction_data, &[0]);
// Set decimals as u8 at offset [1]
write_bytes(&mut instruction_data[1..2], &[self.decimals]);
// Set mint_authority as Pubkey at offset [2..34]
write_bytes(&mut instruction_data[2..34], self.mint_authority);
// Set COption & freeze_authority at offset [34..67]
if let Some(freeze_auth) = self.freeze_authority {
write_bytes(&mut instruction_data[34..35], &[1]);
write_bytes(&mut instruction_data[35..], freeze_auth);
} else {
write_bytes(&mut instruction_data[34..35], &[0]); <-- bytes 35-67 have not been written to
}
let instruction = Instruction {
program_id: &crate::ID,
accounts: &account_metas,
data: unsafe { from_raw_parts(instruction_data.as_ptr() as _, 67) }, <-- unitialized bytes 35-67 are being read from
};
invoke_signed(&instruction, &[self.mint, self.rent_sysvar], signers)
}In the annotated example above, which is taken from InitializeMint, we see that the instruction_data is a slice of UNINIT_BYTE, which is defined as
const UNINIT_BYTE: MaybeUninit<u8> = MaybeUninit::<u8>::uninit();Not all of the uninitialized bytes are written to in both branches of the if statement. In the else branch, bytes 35--67 are not written to and are subsequently read from in the unsafe {} block, leading to undefined behavior.
Impact
This may lead to undefined behavior due to reading from uninitialized memory that was instantiated using the MaybeUninit crate.
Recommendations
Instead of using MaybeUninit, it should be preferred to use a slice of [0u8; X] instead, which will prevent the undefined behavior. Alternatively, changing the size of the slice in response to the amount written will also avoid the undefined behavior.