ink! vulnerable to incorrect decoding of storage value when using `DelegateCall`

@@ -300,7 +300,7 @@ where
 /// - If the called code execution has trapped.
 pub fn invoke_contract_delegate<E, Args, R>(
     params: &CallParams<E, DelegateCall<E>, Args, R>,
-) -> Result<R>
+) -> Result<ink_primitives::MessageResult<R>>
 where
     E: Environment,
     Args: scale::Encode,

@@ -433,7 +433,7 @@ pub trait TypedEnvBackend: EnvBackend {
     fn invoke_contract_delegate<E, Args, R>(
         &mut self,
         call_data: &CallParams<E, DelegateCall<E>, Args, R>,
-    ) -> Result<R>
+    ) -> Result<ink_primitives::MessageResult<R>>
     where
         E: Environment,
         Args: scale::Encode,

@@ -151,13 +151,17 @@ where
     ///
     /// # Panics
     ///
-    /// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]. If you
-    /// want to handle those use the [`try_invoke`][`CallParams::try_invoke`] method
-    /// instead.
+    /// This method panics if it encounters an [`ink::env::Error`][`crate::Error`] or an
+    /// [`ink::primitives::LangError`][`ink_primitives::LangError`]. If you want to handle
+    /// those use the [`try_invoke`][`CallParams::try_invoke`] method instead.
     pub fn invoke(&self) -> R {
-        crate::invoke_contract_delegate(self).unwrap_or_else(|env_error| {
-            panic!("Cross-contract call failed with {env_error:?}")
-        })
+        crate::invoke_contract_delegate(self)
+            .unwrap_or_else(|env_error| {
+                panic!("Cross-contract call failed with {env_error:?}")
+            })
+            .unwrap_or_else(|lang_error| {
+                panic!("Cross-contract call failed with {lang_error:?}")
+            })
     }
 
     /// Invoke the contract using Delegate Call semantics with the given built-up call
@@ -167,9 +171,10 @@ where
     ///
     /// # Note
     ///
-    /// On failure this returns an [`ink::env::Error`][`crate::Error`] which can be
+    /// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
+    /// [`ink::primitives::LangError`][`ink_primitives::LangError`], both of which can be
     /// handled by the caller.
-    pub fn try_invoke(&self) -> Result<R, crate::Error> {
+    pub fn try_invoke(&self) -> Result<ink_primitives::MessageResult<R>, crate::Error> {
         crate::invoke_contract_delegate(self)
     }
 }
@@ -714,7 +719,7 @@ where
     ///
     /// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the
     /// caller.
-    pub fn try_invoke(self) -> Result<(), Error> {
+    pub fn try_invoke(self) -> Result<ink_primitives::MessageResult<()>, Error> {
         self.params().try_invoke()
     }
 }
@@ -761,9 +766,9 @@ where
     ///
     /// # Panics
     ///
-    /// This method panics if it encounters an [`ink::env::Error`][`crate::Error`]
-    /// If you want to handle those use the [`try_invoke`][`CallBuilder::try_invoke`]
-    /// method instead.
+    /// This method panics if it encounters an [`ink::env::Error`][`crate::Error`] or an
+    /// [`ink::primitives::LangError`][`ink_primitives::LangError`]. If you want to handle
+    /// those use the [`try_invoke`][`CallBuilder::try_invoke`] method instead.
     pub fn invoke(self) -> R {
         self.params().invoke()
     }
@@ -773,9 +778,10 @@ where
     ///
     /// # Note
     ///
-    /// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the
-    /// caller.
-    pub fn try_invoke(self) -> Result<R, Error> {
+    /// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
+    /// [`ink::primitives::LangError`][`ink_primitives::LangError`], both of which can be
+    /// handled by the caller.
+    pub fn try_invoke(self) -> Result<ink_primitives::MessageResult<R>, Error> {
         self.params().try_invoke()
     }
 }

@@ -453,7 +453,7 @@ impl TypedEnvBackend for EnvInstance {
     fn invoke_contract_delegate<E, Args, R>(
         &mut self,
         params: &CallParams<E, DelegateCall<E>, Args, R>,
-    ) -> Result<R>
+    ) -> Result<ink_primitives::MessageResult<R>>
     where
         E: Environment,
         Args: scale::Encode,

@@ -444,7 +444,7 @@ impl TypedEnvBackend for EnvInstance {
     fn invoke_contract_delegate<E, Args, R>(
         &mut self,
         params: &CallParams<E, DelegateCall<E>, Args, R>,
-    ) -> Result<R>
+    ) -> Result<ink_primitives::MessageResult<R>>
     where
         E: Environment,
         Args: scale::Encode,

@@ -631,7 +631,7 @@ where
     pub fn invoke_contract_delegate<Args, R>(
         self,
         params: &CallParams<E, DelegateCall<E>, Args, R>,
-    ) -> Result<R>
+    ) -> Result<ink_primitives::MessageResult<R>>
     where
         Args: scale::Encode,
         R: scale::Decode,

@@ -0,0 +1,32 @@
+[package]
+name = "call_builder_delegate"
+version = "4.2.0"
+authors = ["Parity Technologies <admin@parity.io>"]
+edition = "2021"
+publish = false
+
+[dependencies]
+ink = { path = "../../../crates/ink", default-features = false }
+
+scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] }
+scale-info = { version = "2.6", default-features = false, features = ["derive"], optional = true }
+
+incrementer = { path = "../../incrementer", default-features = false, features = ["ink-as-dependency"] }
+
+[dev-dependencies]
+ink_e2e = { path = "../../../crates/e2e" }
+
+[lib]
+path = "lib.rs"
+
+[features]
+default = ["std"]
+std = [
+    "ink/std",
+    "scale/std",
+    "scale-info/std",
+
+    "incrementer/std",
+]
+ink-as-dependency = []
+e2e-tests = []

@@ -0,0 +1,211 @@
+//! # Integration Tests for `LangError`
+//!
+//! This contract is used to ensure that the behavior around `LangError`s works as
+//! expected.
+//!
+//! In particular, it exercises the codepaths that stem from the usage of the
+//! [`CallBuilder`](`ink::env::call::CallBuilder`) and
+//! [`CreateBuilder`](`ink::env::call::CreateBuilder`) structs.
+//!
+//! This differs from the codepath used by external tooling, such as `cargo-contract` or
+//! the `Contracts-UI` which instead depend on methods from the Contracts pallet which are
+//! exposed via RPC.
+//!
+//! Note that during testing we make use of ink!'s end-to-end testing features, so ensure
+//! that you have a node which includes the Contracts pallet running alongside your tests.
+
+#![cfg_attr(not(feature = "std"), no_std, no_main)]
+
+#[ink::contract]
+mod call_builder {
+    use ink::env::{
+        call::{
+            build_call,
+            ExecutionInput,
+            Selector,
+        },
+        DefaultEnvironment,
+    };
+
+    #[ink(storage)]
+    #[derive(Default)]
+    pub struct CallBuilderDelegateTest {
+        /// Since we're going to `DelegateCall` into the `incrementer` contract, we need
+        /// to make sure our storage layout matches.
+        value: i32,
+    }
+
+    impl CallBuilderDelegateTest {
+        #[ink(constructor)]
+        pub fn new(value: i32) -> Self {
+            Self { value }
+        }
+
+        /// Call a contract using the `CallBuilder`.
+        ///
+        /// Since we can't use the `CallBuilder` in a test environment directly we need
+        /// this wrapper to test things like crafting calls with invalid
+        /// selectors.
+        ///
+        /// We also wrap the output in an `Option` since we can't return a `Result`
+        /// directly from a contract message without erroring out ourselves.
+        #[ink(message)]
+        pub fn delegate(
+            &mut self,
+            code_hash: Hash,
+            selector: [u8; 4],
+        ) -> Option<ink::LangError> {
+            let result = build_call::<DefaultEnvironment>()
+                .delegate(code_hash)
+                .exec_input(ExecutionInput::new(Selector::new(selector)))
+                .returns::<bool>()
+                .try_invoke()
+                .expect("Error from the Contracts pallet.");
+
+            match result {
+                Ok(_) => None,
+                Err(e @ ink::LangError::CouldNotReadInput) => Some(e),
+                Err(_) => {
+                    unimplemented!("No other `LangError` variants exist at the moment.")
+                }
+            }
+        }
+
+        /// Call a contract using the `CallBuilder`.
+        ///
+        /// Since we can't use the `CallBuilder` in a test environment directly we need
+        /// this wrapper to test things like crafting calls with invalid
+        /// selectors.
+        ///
+        /// This message does not allow the caller to handle any `LangErrors`, for that
+        /// use the `call` message instead.
+        #[ink(message)]
+        pub fn invoke(&mut self, code_hash: Hash, selector: [u8; 4]) -> i32 {
+            use ink::env::call::build_call;
+
+            build_call::<DefaultEnvironment>()
+                .delegate(code_hash)
+                .exec_input(ExecutionInput::new(Selector::new(selector)))
+                .returns::<i32>()
+                .invoke()
+        }
+    }
+
+    #[cfg(all(test, feature = "e2e-tests"))]
+    mod e2e_tests {
+        use super::*;
+
+        type E2EResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;
+
+        #[ink_e2e::test]
+        async fn e2e_call_builder_delegate_returns_correct_value(
+            mut client: ink_e2e::Client<C, E>,
+        ) -> E2EResult<()> {
+            let origin = client
+                .create_and_fund_account(&ink_e2e::alice(), 10_000_000_000_000)
+                .await;
+
+            let expected_value = 42;
+            let constructor = CallBuilderDelegateTestRef::new(expected_value);
+            let call_builder = client
+                .instantiate("call_builder_delegate", &origin, constructor, 0, None)
+                .await
+                .expect("instantiate failed");
+            let mut call_builder_call = call_builder.call::<CallBuilderDelegateTest>();
+
+            let code_hash = client
+                .upload("incrementer", &origin, None)
+                .await
+                .expect("upload `incrementer` failed")
+                .code_hash;
+
+            let selector = ink::selector_bytes!("get");
+            let call = call_builder_call.invoke(code_hash, selector);
+            let call_result = client
+                .call(&origin, &call, 0, None)
+                .await
+                .expect("Client failed to call `call_builder::invoke`.")
+                .return_value();
+
+            assert_eq!(
+                call_result, expected_value,
+                "Decoded an unexpected value from the call."
+            );
+
+            Ok(())
+        }
+
+        #[ink_e2e::test]
+        async fn e2e_invalid_message_selector_can_be_handled(
+            mut client: ink_e2e::Client<C, E>,
+        ) -> E2EResult<()> {
+            let origin = client
+                .create_and_fund_account(&ink_e2e::bob(), 10_000_000_000_000)
+                .await;
+
+            let constructor = CallBuilderDelegateTestRef::new(Default::default());
+            let call_builder_contract = client
+                .instantiate("call_builder_delegate", &origin, constructor, 0, None)
+                .await
+                .expect("instantiate failed");
+            let mut call_builder_call =
+                call_builder_contract.call::<CallBuilderDelegateTest>();
+
+            let code_hash = client
+                .upload("incrementer", &origin, None)
+                .await
+                .expect("upload `incrementer` failed")
+                .code_hash;
+
+            let selector = ink::selector_bytes!("invalid_selector");
+            let call = call_builder_call.delegate(code_hash, selector);
+            let call_result = client
+                .call(&origin, &call, 0, None)
+                .await
+                .expect("Calling `call_builder::delegate` failed");
+
+            assert!(matches!(
+                call_result.return_value(),
+                Some(ink::LangError::CouldNotReadInput)
+            ));
+
+            Ok(())
+        }
+
+        #[ink_e2e::test]
+        async fn e2e_invalid_message_selector_panics_on_invoke(
+            mut client: ink_e2e::Client<C, E>,
+        ) -> E2EResult<()> {
+            let origin = client
+                .create_and_fund_account(&ink_e2e::charlie(), 10_000_000_000_000)
+                .await;
+
+            let constructor = CallBuilderDelegateTestRef::new(Default::default());
+            let call_builder_contract = client
+                .instantiate("call_builder_delegate", &origin, constructor, 0, None)
+                .await
+                .expect("instantiate failed");
+            let mut call_builder_call =
+                call_builder_contract.call::<CallBuilderDelegateTest>();
+
+            let code_hash = client
+                .upload("incrementer", &origin, None)
+                .await
+                .expect("upload `incrementer` failed")
+                .code_hash;
+
+            // Since `LangError`s can't be handled by the `CallBuilder::invoke()` method
+            // we expect this to panic.
+            let selector = ink::selector_bytes!("invalid_selector");
+            let call = call_builder_call.invoke(code_hash, selector);
+            let call_result = client.call_dry_run(&origin, &call, 0, None).await;
+
+            assert!(call_result.is_err());
+            assert!(call_result
+                .debug_message()
+                .contains("Cross-contract call failed with CouldNotReadInput"));
+
+            Ok(())
+        }
+    }
+}