Polkadot: Prevent storage initializers overwriting the contract input…

… in scratch buf (#1501)

The call to the storage initializer must happen before reading the input into the scratch buffer. Otherwise, the storage initializers can overwrite the input data in the scratch buffer.

src/emit/polkadot/mod.rs

@@ -200,7 +200,8 @@ impl PolkadotTarget {
     fn public_function_prelude<'a>(
         &self,
         binary: &Binary<'a>,
-        function: FunctionValue,
+        function: FunctionValue<'a>,
+        storage_initializer: Option<FunctionValue>,
     ) -> (PointerValue<'a>, IntValue<'a>) {
         let entry = binary.context.append_basic_block(function, "entry");
 
@@ -211,6 +212,11 @@ impl PolkadotTarget {
             .builder
             .build_call(binary.module.get_function("__init_heap").unwrap(), &[], "");
 
+        // Call the storage initializers on deploy
+        if let Some(initializer) = storage_initializer {
+            binary.builder.build_call(initializer, &[], "");
+        }
+
         let scratch_buf = binary.scratch.unwrap().as_pointer_value();
         let scratch_len = binary.scratch_len.unwrap().as_pointer_value();
 
@@ -336,24 +342,29 @@ impl PolkadotTarget {
         external!("set_code_hash", i32_type, u8_ptr);
     }
 
-    /// Emits the "deploy" function if `init` is `Some`, otherwise emits the "call" function.
-    fn emit_dispatch(&mut self, init: Option<FunctionValue>, bin: &mut Binary, ns: &Namespace) {
+    /// Emits the "deploy" function if `storage_initializer` is `Some`, otherwise emits the "call" function.
+    fn emit_dispatch(
+        &mut self,
+        storage_initializer: Option<FunctionValue>,
+        bin: &mut Binary,
+        ns: &Namespace,
+    ) {
         let ty = bin.context.void_type().fn_type(&[], false);
-        let export_name = if init.is_some() { "deploy" } else { "call" };
+        let export_name = if storage_initializer.is_some() {
+            "deploy"
+        } else {
+            "call"
+        };
         let func = bin.module.add_function(export_name, ty, None);
-        let (input, input_length) = self.public_function_prelude(bin, func);
+        let (input, input_length) = self.public_function_prelude(bin, func, storage_initializer);
         let args = vec![
             BasicMetadataValueEnum::PointerValue(input),
             BasicMetadataValueEnum::IntValue(input_length),
             BasicMetadataValueEnum::IntValue(self.value_transferred(bin, ns)),
             BasicMetadataValueEnum::PointerValue(bin.selector.as_pointer_value()),
         ];
-        let dispatch_cfg_name = &init
-            .map(|initializer| {
-                // Call the storage initializers on deploy
-                bin.builder.build_call(initializer, &[], "");
-                DispatchType::Deploy
-            })
+        let dispatch_cfg_name = &storage_initializer
+            .map(|_| DispatchType::Deploy)
             .unwrap_or(DispatchType::Call)
             .to_string();
         let cfg = bin.module.get_function(dispatch_cfg_name).unwrap();

tests/polkadot_tests/storage.rs

@@ -37,3 +37,17 @@ contract foo {
         [SStruct { f1: 1 }, SStruct { f1: 2 }].encode(),
     );
 }
+
+#[test]
+fn storage_initializer_addr_type() {
+    // The contracts storage initializer writes to the scratch buffer in the storage.
+    let mut runtime = build_solidity(r#"contract C { address public owner = msg.sender; }"#);
+
+    // But this must not overwrite the input length; deploy should not revert.
+    runtime.constructor(0, Vec::new());
+    assert!(runtime.output().is_empty());
+
+    // Expect the storage initializer to work properly.
+    runtime.function("owner", Vec::new());
+    assert_eq!(runtime.output(), runtime.caller());
+}