[eventv2] add script event verifier to forbid event emitting

aptos-move/aptos-vm/src/aptos_vm.rs

@@ -426,6 +426,12 @@ impl AptosVM {
                 TransactionPayload::Script(script) => {
                     let loaded_func =
                         session.load_script(script.code(), script.ty_args().to_vec())?;
+                    // Gerardo: consolidate the extended validation to verifier.
+                    verifier::event_validation::verify_no_event_emission_in_script(
+                        script.code(),
+                        session.get_vm_config().max_binary_format_version,
+                    )?;
+
                     let args =
                         verifier::transaction_arg_validation::validate_combine_signer_and_txn_args(
                             &mut session,

aptos-move/aptos-vm/src/verifier/event_validation.rs

@@ -4,10 +4,10 @@
 use crate::move_vm_ext::SessionExt;
 use aptos_framework::RuntimeModuleMetadataV1;
 use move_binary_format::{
-    access::ModuleAccess,
+    access::{ModuleAccess, ScriptAccess},
     errors::{Location, PartialVMError, VMError, VMResult},
     file_format::{
-        Bytecode,
+        Bytecode, CompiledScript,
         SignatureToken::{Struct, StructInstantiation},
     },
     CompiledModule,
@@ -25,12 +25,12 @@ fn metadata_validation_err(msg: &str) -> Result<(), VMError> {
 }
 
 fn metadata_validation_error(msg: &str) -> VMError {
-    PartialVMError::new(StatusCode::CONSTRAINT_NOT_SATISFIED)
+    PartialVMError::new(StatusCode::EVENT_METADATA_VALIDATION_ERROR)
         .with_message(format!("metadata and code bundle mismatch: {}", msg))
         .finish(Location::Undefined)
 }
 
-/// Validate resource group metadata on modules one by one:
+/// Validate event metadata on modules one by one:
 /// * Extract the event metadata
 /// * Verify all changes are compatible upgrades (existing event attributes cannot be removed)
 pub(crate) fn validate_module_events(
@@ -77,8 +77,7 @@ pub(crate) fn validate_emit_calls(
                     let module_addr = module.address_identifier_at(module_handle.address);
                     let module_name = module.identifier_at(module_handle.name);
                     let func_name = module.identifier_at(func_handle.name);
-                    if module_addr
-                        != &AccountAddress::from_hex_literal("0x1").expect("valid address")
+                    if module_addr != &AccountAddress::ONE
                         || module_name.as_str() != EVENT_MODULE_NAME
                         || func_name.as_str() != EVENT_EMIT_FUNCTION_NAME
                     {
@@ -147,3 +146,39 @@ pub(crate) fn extract_event_metadata(
     }
     Ok(event_structs)
 }
+
+pub(crate) fn verify_no_event_emission_in_script(
+    script_code: &[u8],
+    max_binary_format_version: u32,
+) -> VMResult<()> {
+    let script = match CompiledScript::deserialize_with_max_version(
+        script_code,
+        max_binary_format_version,
+    ) {
+        Ok(script) => script,
+        Err(err) => {
+            let msg = format!("[VM] deserializer for script returned error: {:?}", err);
+            return Err(PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR)
+                .with_message(msg)
+                .finish(Location::Script));
+        },
+    };
+    for bc in &script.code().code {
+        if let Bytecode::CallGeneric(index) = bc {
+            let func_instantiation = &script.function_instantiation_at(*index);
+            let func_handle = script.function_handle_at(func_instantiation.handle);
+            let module_handle = script.module_handle_at(func_handle.module);
+            let module_addr = script.address_identifier_at(module_handle.address);
+            let module_name = script.identifier_at(module_handle.name);
+            let func_name = script.identifier_at(func_handle.name);
+            if module_addr == &AccountAddress::ONE
+                && module_name.as_str() == EVENT_MODULE_NAME
+                && func_name.as_str() == EVENT_EMIT_FUNCTION_NAME
+            {
+                return Err(PartialVMError::new(StatusCode::INVALID_OPERATION_IN_SCRIPT)
+                    .finish(Location::Script));
+            }
+        }
+    }
+    Ok(())
+}

aptos-move/e2e-testsuite/src/tests/scripts.rs

@@ -9,8 +9,9 @@ use aptos_types::{
     transaction::{ExecutionStatus, Script, TransactionStatus},
 };
 use move_binary_format::file_format::{
-    empty_script, AbilitySet, AddressIdentifierIndex, Bytecode, FunctionHandle,
-    FunctionHandleIndex, IdentifierIndex, ModuleHandle, ModuleHandleIndex, SignatureIndex,
+    empty_script, Ability, AbilitySet, AddressIdentifierIndex, Bytecode, FunctionHandle,
+    FunctionHandleIndex, FunctionInstantiation, FunctionInstantiationIndex, IdentifierIndex,
+    ModuleHandle, ModuleHandleIndex, Signature, SignatureIndex, SignatureToken,
 };
 use move_core_types::{
     identifier::Identifier,
@@ -434,3 +435,83 @@ fn script_nested_type_argument_module_does_not_exist() {
     assert_eq!(balance, updated_sender_balance.coin());
     assert_eq!(11, updated_sender.sequence_number());
 }
+
+#[test]
+fn forbid_script_emitting_events() {
+    let mut executor = FakeExecutor::from_head_genesis();
+
+    // create and publish sender
+    let sender = executor.create_raw_account_data(1_000_000, 10);
+    executor.add_account_data(&sender);
+
+    // create an event-emitting script
+    let mut script = empty_script();
+    script.code.code = vec![
+        Bytecode::LdTrue,
+        Bytecode::CallGeneric(FunctionInstantiationIndex(0)),
+        Bytecode::Ret,
+    ];
+    script.function_instantiations.push(FunctionInstantiation {
+        handle: FunctionHandleIndex(0),
+        type_parameters: SignatureIndex(2),
+    });
+    script.function_handles.push(FunctionHandle {
+        module: ModuleHandleIndex(0),
+        name: IdentifierIndex(1),
+        parameters: SignatureIndex(1),
+        return_: SignatureIndex(0),
+        type_parameters: vec![
+            AbilitySet::singleton(Ability::Store) | AbilitySet::singleton(Ability::Drop),
+        ],
+    });
+    script.module_handles.push(ModuleHandle {
+        address: AddressIdentifierIndex(0),
+        name: IdentifierIndex(0),
+    });
+    script.address_identifiers.push(AccountAddress::ONE);
+    script.identifiers = vec![
+        Identifier::new("event").unwrap(),
+        Identifier::new("emit").unwrap(),
+    ];
+    // dummy signatures
+    script.signatures = vec![
+        Signature(vec![]),
+        Signature(vec![SignatureToken::TypeParameter(0)]),
+        Signature(vec![SignatureToken::Bool]),
+    ];
+    let mut blob = vec![];
+    script.serialize(&mut blob).expect("script must serialize");
+    let txn = sender
+        .account()
+        .transaction()
+        .script(Script::new(blob, vec![], vec![]))
+        .sequence_number(10)
+        .gas_unit_price(1)
+        .sign();
+    // execute transaction
+    let output = &executor.execute_transaction(txn);
+    let status = output.status();
+    match status {
+        TransactionStatus::Keep(_) => (),
+        _ => panic!("TransactionStatus must be Keep"),
+    }
+    assert_eq!(
+        status.status(),
+        Ok(ExecutionStatus::MiscellaneousError(Some(
+            StatusCode::INVALID_OPERATION_IN_SCRIPT
+        )))
+    );
+    executor.apply_write_set(output.write_set());
+
+    // Check that numbers in store are correct.
+    let gas = output.gas_used();
+    let balance = 1_000_000 - gas;
+    let updated_sender = executor
+        .read_account_resource(sender.account())
+        .expect("sender must exist");
+    let updated_sender_balance = executor
+        .read_coin_store_resource(sender.account())
+        .expect("sender balance must exist");
+    assert_eq!(balance, updated_sender_balance.coin());
+    assert_eq!(11, updated_sender.sequence_number());
+}

aptos-move/framework/src/natives/event.rs

@@ -13,8 +13,6 @@ use aptos_types::contract_event::ContractEvent;
 use aptos_types::event::EventKey;
 use better_any::{Tid, TidAble};
 use move_binary_format::errors::PartialVMError;
-#[cfg(feature = "testing")]
-use move_binary_format::errors::PartialVMResult;
 use move_core_types::{language_storage::TypeTag, vm_status::StatusCode};
 use move_vm_runtime::native_functions::NativeFunction;
 #[cfg(feature = "testing")]
@@ -35,11 +33,7 @@ impl NativeEventContext {
     }
 
     #[cfg(feature = "testing")]
-    fn emitted_v1_events(
-        &self,
-        event_key: &EventKey,
-        ty_tag: &TypeTag,
-    ) -> PartialVMResult<Vec<&[u8]>> {
+    fn emitted_v1_events(&self, event_key: &EventKey, ty_tag: &TypeTag) -> Vec<&[u8]> {
         let mut events = vec![];
         for event in self.events.iter() {
             if let ContractEvent::V1(e) = event {
@@ -48,11 +42,11 @@ impl NativeEventContext {
                 }
             }
         }
-        Ok(events)
+        events
     }
 
     #[cfg(feature = "testing")]
-    fn emitted_v2_events(&self, ty_tag: &TypeTag) -> PartialVMResult<Vec<&[u8]>> {
+    fn emitted_v2_events(&self, ty_tag: &TypeTag) -> Vec<&[u8]> {
         let mut events = vec![];
         for event in self.events.iter() {
             if let ContractEvent::V2(e) = event {
@@ -61,7 +55,7 @@ impl NativeEventContext {
                 }
             }
         }
-        Ok(events)
+        events
     }
 }
 
@@ -94,7 +88,7 @@ fn native_write_to_event_store(
     let ty_layout = context.type_to_type_layout(&ty)?;
     let blob = msg.simple_serialize(&ty_layout).ok_or_else(|| {
         SafeNativeError::InvariantViolation(PartialVMError::new(
-            StatusCode::VALUE_DESERIALIZATION_ERROR,
+            StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
         ))
     })?;
     let key = bcs::from_bytes(guid.as_slice()).map_err(|_| {
@@ -147,12 +141,12 @@ fn native_emitted_events_by_handle(
     let ty_layout = context.type_to_type_layout(&ty)?;
     let ctx = context.extensions_mut().get_mut::<NativeEventContext>();
     let events = ctx
-        .emitted_v1_events(&key, &ty_tag)?
+        .emitted_v1_events(&key, &ty_tag)
         .into_iter()
         .map(|blob| {
             Value::simple_deserialize(blob, &ty_layout).ok_or_else(|| {
                 SafeNativeError::InvariantViolation(PartialVMError::new(
-                    StatusCode::VALUE_DESERIALIZATION_ERROR,
+                    StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
                 ))
             })
         })
@@ -175,7 +169,7 @@ fn native_emitted_events(
     let ty_layout = context.type_to_type_layout(&ty)?;
     let ctx = context.extensions_mut().get_mut::<NativeEventContext>();
     let events = ctx
-        .emitted_v2_events(&ty_tag)?
+        .emitted_v2_events(&ty_tag)
         .into_iter()
         .map(|blob| {
             Value::simple_deserialize(blob, &ty_layout).ok_or_else(|| {
@@ -207,7 +201,7 @@ fn native_write_module_event_to_store(
 
     let type_tag = context.type_to_type_tag(&ty)?;
 
-    // Runtime check for script/module call.
+    // Additional runtime check for module call.
     if let (Some(id), _, _) = context
         .stack_frames(1)
         .stack_trace()
@@ -229,15 +223,11 @@ fn native_write_module_event_to_store(
                 StatusCode::INTERNAL_TYPE_ERROR,
             )));
         }
-    } else {
-        return Err(SafeNativeError::InvariantViolation(PartialVMError::new(
-            StatusCode::INVALID_OPERATION_IN_SCRIPT,
-        )));
     }
     let layout = context.type_to_type_layout(&ty)?;
     let blob = msg.simple_serialize(&layout).ok_or_else(|| {
         SafeNativeError::InvariantViolation(
-            PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR)
+            PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR)
                 .with_message("Event serialization failure".to_string()),
         )
     })?;

third_party/move/move-bytecode-verifier/src/dependencies.rs

@@ -252,7 +252,7 @@ fn verify_imported_structs(context: &Context) -> PartialVMResult<()> {
                     StatusCode::LOOKUP_FAILED,
                     IndexKind::StructHandle,
                     idx as TableIndex,
-                ))
+                ));
             },
         }
     }

third_party/move/move-core/types/src/vm_status.rs

@@ -701,7 +701,7 @@ pub enum StatusCode {
     MAX_FIELD_DEFINITIONS_REACHED = 1121,
     // Reserved error code for future use
     TOO_MANY_BACK_EDGES = 1122,
-    RESERVED_VERIFICATION_ERROR_1 = 1123,
+    EVENT_METADATA_VALIDATION_ERROR = 1123,
     RESERVED_VERIFICATION_ERROR_2 = 1124,
     RESERVED_VERIFICATION_ERROR_3 = 1125,
     RESERVED_VERIFICATION_ERROR_4 = 1126,

third_party/move/move-vm/runtime/src/session.rs

@@ -3,7 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{
-    data_cache::TransactionDataCache, loader::LoadedFunction, move_vm::MoveVM,
+    config::VMConfig, data_cache::TransactionDataCache, loader::LoadedFunction, move_vm::MoveVM,
     native_extensions::NativeContextExtensions,
 };
 use move_binary_format::{
@@ -422,6 +422,10 @@ impl<'r, 'l> Session<'r, 'l> {
     pub fn get_move_vm(&self) -> &'l MoveVM {
         self.move_vm
     }
+
+    pub fn get_vm_config(&self) -> &'l VMConfig {
+        self.move_vm.runtime.loader().vm_config()
+    }
 }
 
 pub struct LoadedFunctionInstantiation {