Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Compiler-v2][VM] add metadata check for script #14099

Merged
merged 2 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ pub enum FeatureFlag {
UseCompatibilityCheckerV2,
EnableEnumTypes,
EnableResourceAccessControl,
RejectUnstableBytecodeForScript,
}

fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
Expand Down Expand Up @@ -332,6 +333,9 @@ impl From<FeatureFlag> for AptosFeatureFlag {
FeatureFlag::EnableResourceAccessControl => {
AptosFeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL
},
FeatureFlag::RejectUnstableBytecodeForScript => {
AptosFeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT
},
}
}
}
Expand Down Expand Up @@ -467,6 +471,9 @@ impl From<AptosFeatureFlag> for FeatureFlag {
AptosFeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL => {
FeatureFlag::EnableResourceAccessControl
},
AptosFeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT => {
FeatureFlag::RejectUnstableBytecodeForScript
},
}
}
}
Expand Down
42 changes: 39 additions & 3 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
compatibility::Compatibility,
deserializer::DeserializerConfig,
errors::{Location, PartialVMError, PartialVMResult, VMError, VMResult},
file_format::CompiledScript,
CompiledModule,
};
use move_core_types::{
Expand Down Expand Up @@ -738,11 +739,30 @@

let func = session.load_script(script.code(), script.ty_args())?;

// TODO(Gerardo): consolidate the extended validation to verifier.
verifier::event_validation::verify_no_event_emission_in_script(
let compiled_script = match CompiledScript::deserialize_with_config(

Check warning on line 742 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L742 was not covered by tests
script.code(),
self.deserializer_config(),
)?;
) {
Ok(script) => script,
Err(err) => {
let msg = format!("[VM] deserializer for script returned error: {:?}", err);
let partial_err = PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR)
.with_message(msg)
.finish(Location::Script);
return Err(partial_err.into_vm_status());

Check warning on line 752 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L745-L752

Added lines #L745 - L752 were not covered by tests
},
};

// Check that unstable bytecode cannot be executed on mainnet
if self
.features()
.is_enabled(FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT)

Check warning on line 759 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L757-L759

Added lines #L757 - L759 were not covered by tests
georgemitenkov marked this conversation as resolved.
Show resolved Hide resolved
{
self.reject_unstable_bytecode_for_script(&compiled_script)?;
}

Check warning on line 762 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L761-L762

Added lines #L761 - L762 were not covered by tests

// TODO(Gerardo): consolidate the extended validation to verifier.
verifier::event_validation::verify_no_event_emission_in_compiled_script(&compiled_script)?;

Check warning on line 765 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L765 was not covered by tests

let args = verifier::transaction_arg_validation::validate_combine_signer_and_txn_args(
session,
Expand Down Expand Up @@ -1622,6 +1642,22 @@
Ok(())
}

/// Check whether the script can be run on mainnet based on the unstable tag in the metadata
pub fn reject_unstable_bytecode_for_script(&self, module: &CompiledScript) -> VMResult<()> {
if self.chain_id().is_mainnet() {
if let Some(metadata) =
aptos_framework::get_compilation_metadata_from_compiled_script(module)

Check warning on line 1649 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1646-L1649

Added lines #L1646 - L1649 were not covered by tests
{
if metadata.unstable {
return Err(PartialVMError::new(StatusCode::UNSTABLE_BYTECODE_REJECTED)
.with_message("script marked unstable cannot be run on mainnet".to_string())
.finish(Location::Script));
}
}
}
Ok(())
}

Check warning on line 1659 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1651-L1659

Added lines #L1651 - L1659 were not covered by tests

fn metadata_validation_error(msg: &str) -> VMError {
PartialVMError::new(StatusCode::CONSTRAINT_NOT_SATISFIED)
.with_message(format!("metadata and code bundle mismatch: {}", msg))
Expand Down
15 changes: 1 addition & 14 deletions aptos-move/aptos-vm/src/verifier/event_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use aptos_framework::RuntimeModuleMetadataV1;
use move_binary_format::{
access::{ModuleAccess, ScriptAccess},
deserializer::DeserializerConfig,
errors::{Location, PartialVMError, VMError, VMResult},
file_format::{
Bytecode, CompiledScript,
Expand Down Expand Up @@ -151,19 +150,7 @@
Ok(event_structs)
}

pub(crate) fn verify_no_event_emission_in_script(
script_code: &[u8],
config: &DeserializerConfig,
) -> VMResult<()> {
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);
return Err(PartialVMError::new(StatusCode::CODE_DESERIALIZATION_ERROR)
.with_message(msg)
.finish(Location::Script));
},
};
pub(crate) fn verify_no_event_emission_in_compiled_script(script: &CompiledScript) -> VMResult<()> {

Check warning on line 153 in aptos-move/aptos-vm/src/verifier/event_validation.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L153 was not covered by tests
for bc in &script.code().code {
if let Bytecode::CallGeneric(index) = bc {
let func_instantiation = &script.function_instantiation_at(*index);
Expand Down
114 changes: 113 additions & 1 deletion aptos-move/e2e-move-tests/src/tests/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use aptos_package_builder::PackageBuilder;
use aptos_types::{
chain_id::ChainId,
on_chain_config::{FeatureFlag, OnChainConfig},
transaction::TransactionStatus,
transaction::{Script, TransactionPayload, TransactionStatus},
};
use move_binary_format::CompiledModule;
use move_core_types::{
Expand Down Expand Up @@ -269,6 +269,118 @@ fn test_compilation_metadata_internal(
}
}

fn test_compilation_metadata_script_internal(
mainnet_flag: bool,
v2_flag: bool,
feature_enabled: bool,
) -> TransactionStatus {
let mut h = MoveHarness::new();
if feature_enabled {
h.enable_features(
vec![FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT],
vec![],
);
} else {
h.enable_features(vec![], vec![
FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT,
]);
}
let account = h.new_account_at(AccountAddress::from_hex_literal("0xf00d").unwrap());
let mut builder = PackageBuilder::new("Package");
builder.add_source(
"m.move",
r#"
script {
fun main() { }
}
"#,
);
let path = builder.write_to_temp().unwrap();

let compiler_version = if v2_flag {
CompilerVersion::V2_0
} else {
CompilerVersion::V1
};
let package = build_package_with_compiler_version(
path.path().to_path_buf(),
BuildOptions::default(),
compiler_version,
)
.expect("building package must succeed");

let code = package.extract_script_code().into_iter().next().unwrap();

let script = TransactionPayload::Script(Script::new(code, vec![], vec![]));

if mainnet_flag {
h.set_resource(
CORE_CODE_ADDRESS,
ChainId::struct_tag(),
&ChainId::mainnet().id(),
);
h.run_transaction_payload_mainnet(&account, script)
} else {
h.run_transaction_payload(&account, script)
}
}

#[test]
fn test_compilation_metadata_for_script() {
let mut enable_check = true;
// run compiler v2 code to mainnet
assert_vm_status!(
test_compilation_metadata_script_internal(true, true, enable_check),
StatusCode::UNSTABLE_BYTECODE_REJECTED
);
// run compiler v1 code to mainnet
assert_success!(test_compilation_metadata_script_internal(
true,
false,
enable_check
));
// run compiler v2 code to test
assert_success!(test_compilation_metadata_script_internal(
false,
true,
enable_check
));
// run compiler v1 code to test
assert_success!(test_compilation_metadata_script_internal(
false,
false,
enable_check
));

enable_check = false;
// run compiler v2 code to mainnet
// success because the feature flag is turned off
assert_success!(test_compilation_metadata_script_internal(
true,
true,
enable_check
),);
// run compiler v1 code to mainnet
assert_success!(test_compilation_metadata_script_internal(
true,
false,
enable_check
));
// run compiler v2 code to test
// success because the feature flag is turned off
assert_success!(test_compilation_metadata_script_internal(
false,
true,
enable_check
),);
// run compiler v1 code to test
assert_success!(test_compilation_metadata_script_internal(
false,
false,
enable_check
));
}

#[test]
fn test_compilation_metadata() {
let mut enable_check = true;
Expand Down
11 changes: 11 additions & 0 deletions aptos-move/framework/src/module_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,17 @@
}
}

/// Extract compilation metadata from a compiled script
pub fn get_compilation_metadata_from_compiled_script(
module: &CompiledScript,
) -> Option<CompilationMetadata> {
if let Some(data) = find_metadata_in_script(module, COMPILATION_METADATA_KEY) {
bcs::from_bytes::<CompilationMetadata>(&data.value).ok()

Check warning on line 320 in aptos-move/framework/src/module_metadata.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/src/module_metadata.rs#L316-L320

Added lines #L316 - L320 were not covered by tests
} else {
None

Check warning on line 322 in aptos-move/framework/src/module_metadata.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/src/module_metadata.rs#L322

Added line #L322 was not covered by tests
}
}

Check warning on line 324 in aptos-move/framework/src/module_metadata.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/src/module_metadata.rs#L324

Added line #L324 was not covered by tests

// This is mostly a copy paste of the existing function
// get_metadata_from_compiled_module. In the API types there is a unifying trait for
// modules and scripts called Bytecode that could help eliminate this duplication,
Expand Down
2 changes: 2 additions & 0 deletions types/src/on_chain_config/aptos_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ pub enum FeatureFlag {
USE_COMPATIBILITY_CHECKER_V2 = 73,
ENABLE_ENUM_TYPES = 74,
ENABLE_RESOURCE_ACCESS_CONTROL = 75,
REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT = 76,
}

impl FeatureFlag {
Expand Down Expand Up @@ -165,6 +166,7 @@ impl FeatureFlag {
FeatureFlag::USE_COMPATIBILITY_CHECKER_V2,
FeatureFlag::ENABLE_ENUM_TYPES,
FeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL,
FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT,
]
}
}
Expand Down
Loading