Limit the maximum identifier length (aptos-labs#10502)

aptos-move/aptos-release-builder/src/components/feature_flags.rs

@@ -88,6 +88,7 @@ pub enum FeatureFlag {
     FeePayerAccountOptional,
     AggregatorV2DelayedFields,
     ConcurrentAssets,
+    LimitMaxIdentifierLength,
 }
 
 fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
@@ -230,6 +231,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
                 AptosFeatureFlag::AGGREGATOR_V2_DELAYED_FIELDS
             },
             FeatureFlag::ConcurrentAssets => AptosFeatureFlag::CONCURRENT_ASSETS,
+            FeatureFlag::LimitMaxIdentifierLength => AptosFeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH,
         }
     }
 }
@@ -295,6 +297,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
                 FeatureFlag::AggregatorV2DelayedFields
             },
             AptosFeatureFlag::CONCURRENT_ASSETS => FeatureFlag::ConcurrentAssets,
+            AptosFeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH => FeatureFlag::LimitMaxIdentifierLength,
         }
     }
 }

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

@@ -57,7 +57,9 @@ use fail::fail_point;
 use move_binary_format::{
     access::ModuleAccess,
     compatibility::Compatibility,
+    deserializer::DeserializerConfig,
     errors::{verification_error, Location, PartialVMError, VMError, VMResult},
+    file_format_common::{IDENTIFIER_SIZE_MAX, LEGACY_IDENTIFIER_SIZE_MAX},
     CompiledModule, IndexKind,
 };
 use move_core_types::{
@@ -462,7 +464,7 @@ impl AptosVM {
                     // 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,
+                        &session.get_vm_config().deserializer_config,
                     )?;
 
                     let args =
@@ -785,7 +787,10 @@ impl AptosVM {
         module_bundle: &ModuleBundle,
     ) -> VMResult<()> {
         for module_blob in module_bundle.iter() {
-            match CompiledModule::deserialize(module_blob.code()) {
+            match CompiledModule::deserialize_with_config(
+                module_blob.code(),
+                &session.get_vm_config().deserializer_config,
+            ) {
                 Ok(module) => {
                     // verify the module doesn't exist
                     if session.load_module(&module.self_id()).is_ok() {
@@ -859,9 +864,19 @@ impl AptosVM {
         } else {
             5
         };
+        let max_identifier_size = if self
+            .0
+            .get_features()
+            .is_enabled(FeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH)
+        {
+            IDENTIFIER_SIZE_MAX
+        } else {
+            LEGACY_IDENTIFIER_SIZE_MAX
+        };
+        let config = DeserializerConfig::new(max_version, max_identifier_size);
         let mut result = vec![];
         for module_blob in modules.iter() {
-            match CompiledModule::deserialize_with_max_version(module_blob.code(), max_version) {
+            match CompiledModule::deserialize_with_config(module_blob.code(), &config) {
                 Ok(module) => {
                     result.push(module);
                 },

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

@@ -6,8 +6,8 @@
 use crate::{
     aptos_vm_impl::gas_config,
     move_vm_ext::{
-        get_max_binary_format_version, AptosMoveResolver, AsExecutorView, AsResourceGroupView,
-        ResourceGroupResolver,
+        get_max_binary_format_version, get_max_identifier_size, AptosMoveResolver, AsExecutorView,
+        AsResourceGroupView, ResourceGroupResolver,
     },
 };
 #[allow(unused_imports)]
@@ -33,7 +33,7 @@ use aptos_vm_types::{
     resource_group_adapter::ResourceGroupAdapter,
 };
 use bytes::Bytes;
-use move_binary_format::{errors::*, CompiledModule};
+use move_binary_format::{deserializer::DeserializerConfig, errors::*, CompiledModule};
 use move_core_types::{
     account_address::AccountAddress,
     language_storage::{ModuleId, StructTag},
@@ -76,7 +76,7 @@ impl<'a> ConfigStorage for ConfigAdapter<'a, StateKey, MoveTypeLayout> {
 /// for (non-group) resources and subsequent handling in the StorageAdapter itself.
 pub struct StorageAdapter<'e, E> {
     executor_view: &'e E,
-    max_binary_format_version: u32,
+    deserializer_config: DeserializerConfig,
     resource_group_view: ResourceGroupAdapter<'e>,
     accessed_groups: RefCell<HashSet<StateKey>>,
 }
@@ -89,13 +89,19 @@ impl<'e, E: ExecutorView> StorageAdapter<'e, E> {
         maybe_resource_group_view: Option<&'e dyn ResourceGroupView>,
     ) -> Self {
         let max_binary_version = get_max_binary_format_version(features, gas_feature_version);
+        let max_identifier_size = get_max_identifier_size(features);
         let resource_group_adapter = ResourceGroupAdapter::new(
             maybe_resource_group_view,
             executor_view,
             gas_feature_version,
         );
 
-        Self::new(executor_view, max_binary_version, resource_group_adapter)
+        Self::new(
+            executor_view,
+            max_binary_version,
+            max_identifier_size,
+            resource_group_adapter,
+        )
     }
 
     // TODO(gelash, georgemitenkov): delete after simulation uses block executor.
@@ -110,11 +116,15 @@ impl<'e, E: ExecutorView> StorageAdapter<'e, E> {
     fn new(
         executor_view: &'e E,
         max_binary_format_version: u32,
+        max_identifier_size: u64,
         resource_group_view: ResourceGroupAdapter<'e>,
     ) -> Self {
         Self {
             executor_view,
-            max_binary_format_version,
+            deserializer_config: DeserializerConfig::new(
+                max_binary_format_version,
+                max_identifier_size,
+            ),
             resource_group_view,
             accessed_groups: RefCell::new(HashSet::new()),
         }
@@ -221,13 +231,12 @@ impl<'e, E: ExecutorView> ModuleResolver for StorageAdapter<'e, E> {
             Ok(Some(bytes)) => bytes,
             _ => return vec![],
         };
-        let module = match CompiledModule::deserialize_with_max_version(
-            &module_bytes,
-            self.max_binary_format_version,
-        ) {
-            Ok(module) => module,
-            _ => return vec![],
-        };
+        let module =
+            match CompiledModule::deserialize_with_config(&module_bytes, &self.deserializer_config)
+            {
+                Ok(module) => module,
+                _ => return vec![],
+            };
         module.metadata
     }
 
@@ -286,8 +295,13 @@ impl<S: StateView> AsMoveResolver<S> for S {
         let features = Features::fetch_config(&config_view).unwrap_or_default();
         let max_binary_version = get_max_binary_format_version(&features, gas_feature_version);
         let resource_group_adapter = ResourceGroupAdapter::new(None, self, gas_feature_version);
-
-        StorageAdapter::new(self, max_binary_version, resource_group_adapter)
+        let max_identifier_size = get_max_identifier_size(&features);
+        StorageAdapter::new(
+            self,
+            max_binary_version,
+            max_identifier_size,
+            resource_group_adapter,
+        )
     }
 }
 
@@ -357,6 +371,6 @@ pub(crate) mod tests {
             GroupSizeKind::None => 1,
         };
         let group_adapter = ResourceGroupAdapter::new(None, state_view, gas_feature_version);
-        StorageAdapter::new(state_view, 0, group_adapter)
+        StorageAdapter::new(state_view, 0, 0, group_adapter)
     }
 }

aptos-move/aptos-vm/src/move_vm_ext/mod.rs

@@ -14,5 +14,5 @@ pub use crate::move_vm_ext::{
     resolver::{AptosMoveResolver, AsExecutorView, AsResourceGroupView, ResourceGroupResolver},
     respawned_session::RespawnedSession,
     session::{SessionExt, SessionId},
-    vm::{get_max_binary_format_version, verifier_config, MoveVmExt},
+    vm::{get_max_binary_format_version, get_max_identifier_size, verifier_config, MoveVmExt},
 };

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

@@ -15,7 +15,11 @@ use aptos_gas_schedule::{MiscGasParameters, NativeGasParameters};
 use aptos_native_interface::SafeNativeBuilder;
 use aptos_table_natives::NativeTableContext;
 use aptos_types::on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, TimedFeatures};
-use move_binary_format::errors::VMResult;
+use move_binary_format::{
+    deserializer::DeserializerConfig,
+    errors::VMResult,
+    file_format_common::{IDENTIFIER_SIZE_MAX, LEGACY_IDENTIFIER_SIZE_MAX},
+};
 use move_bytecode_verifier::VerifierConfig;
 use move_vm_runtime::{
     config::VMConfig, move_vm::MoveVM, native_extensions::NativeContextExtensions,
@@ -36,6 +40,14 @@ pub fn get_max_binary_format_version(features: &Features, gas_feature_version: u
     }
 }
 
+pub fn get_max_identifier_size(features: &Features) -> u64 {
+    if features.is_enabled(FeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH) {
+        IDENTIFIER_SIZE_MAX
+    } else {
+        LEGACY_IDENTIFIER_SIZE_MAX
+    }
+}
+
 impl MoveVmExt {
     fn new_impl<F>(
         native_gas_params: NativeGasParameters,
@@ -56,6 +68,8 @@ impl MoveVmExt {
         let max_binary_format_version =
             get_max_binary_format_version(&features, gas_feature_version);
 
+        let max_identifier_size = get_max_identifier_size(&features);
+
         let enable_invariant_violation_check_in_swap_loc =
             !timed_features.is_enabled(TimedFeatureFlag::DisableInvariantViolationCheckInSwapLoc);
         let type_size_limit = true;
@@ -89,7 +103,10 @@ impl MoveVmExt {
                 builder,
                 VMConfig {
                     verifier: verifier_config,
-                    max_binary_format_version,
+                    deserializer_config: DeserializerConfig::new(
+                        max_binary_format_version,
+                        max_identifier_size,
+                    ),
                     paranoid_type_checks: crate::AptosVM::get_paranoid_checks(),
                     enable_invariant_violation_check_in_swap_loc,
                     type_size_limit,

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

@@ -5,6 +5,7 @@ use crate::move_vm_ext::SessionExt;
 use aptos_framework::RuntimeModuleMetadataV1;
 use move_binary_format::{
     access::{ModuleAccess, ScriptAccess},
+    deserializer::DeserializerConfig,
     errors::{Location, PartialVMError, VMError, VMResult},
     file_format::{
         Bytecode, CompiledScript,
@@ -121,8 +122,11 @@ pub(crate) fn extract_event_metadata_from_module(
     module_id: &ModuleId,
 ) -> VMResult<HashSet<String>> {
     let metadata = session.load_module(module_id).map(|module| {
-        CompiledModule::deserialize(&module)
-            .map(|module| aptos_framework::get_metadata_from_compiled_module(&module))
+        CompiledModule::deserialize_with_config(
+            &module,
+            &session.get_vm_config().deserializer_config,
+        )
+        .map(|module| aptos_framework::get_metadata_from_compiled_module(&module))
     });
 
     if let Ok(Ok(Some(metadata))) = metadata {
@@ -149,12 +153,9 @@ pub(crate) fn extract_event_metadata(
 
 pub(crate) fn verify_no_event_emission_in_script(
     script_code: &[u8],
-    max_binary_format_version: u32,
+    config: &DeserializerConfig,
 ) -> VMResult<()> {
-    let script = match CompiledScript::deserialize_with_max_version(
-        script_code,
-        max_binary_format_version,
-    ) {
+    let script = match CompiledScript::deserialize_with_config(script_code, config) {
         Ok(script) => script,
         Err(err) => {
             let msg = format!("[VM] deserializer for script returned error: {:?}", err);

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

@@ -151,9 +151,12 @@ pub(crate) fn extract_resource_group_metadata_from_module(
     BTreeMap<String, StructTag>,
     BTreeSet<String>,
 )> {
-    let module = session
-        .load_module(module_id)
-        .map(|module| CompiledModule::deserialize(&module));
+    let module = session.load_module(module_id).map(|module| {
+        CompiledModule::deserialize_with_config(
+            &module,
+            &session.get_vm_config().deserializer_config,
+        )
+    });
     let (metadata, module) = if let Ok(Ok(module)) = module {
         (
             aptos_framework::get_metadata_from_compiled_module(&module),

aptos-move/e2e-move-tests/src/tests/mod.rs

@@ -40,7 +40,6 @@ mod storage_refund;
 mod string_args;
 mod token_event_store;
 mod token_objects;
-mod too_large;
 mod transaction_fee;
 mod type_too_large;
 mod vector_numeric_address;