CVE-2023-34449 (Rust, published 2023-06-14T21:15Z)
Description: value: ink! is an embedded domain specific language to write smart contracts in Rust for blockchains built on the Substrate framework. Starting in version 4.0.0 and prior to version 4.2.1, the return value when using delegate call mechanics, either through `CallBuilder::delegate` or `ink_env::invoke_contract_delegate`, is decoded incorrectly. This bug was related to the mechanics around decoding a call's return buffer, which was changed as part of pull request 1450. Since this feature was only released in ink! 4.0.0, no previous versions are affected. Users who have an ink! 4.x series contract should upgrade to 4.2.1 to receive a patch.
Severity: nan (CVSS v3 base score: 5.3)
Weakness: CWE-253 \u2014 Incorrect Check of Function Return Value
The product incorrectly checks a return value from a function, which prevents it from detecting errors or exceptional conditions.
Repository: https://github.com/paritytech/ink
Files changed:
crates/env/src/api.rs
crates/env/src/backend.rs
crates/env/src/call/call_builder.rs
crates/env/src/engine/off_chain/impls.rs
crates/env/src/engine/on_chain/impls.rs
crates/ink/src/env_access.rs
integration-tests/lang-err-integration-tests/call-builder-delegate/Cargo.toml
integration-tests/lang-err-integration-tests/call-builder-delegate/lib.rs
Commit message: Merge pull request from GHSA-853p-5678-hv8f

* Handle `LangError` from `DelegateCall`

* Add basic delegate E2E test

* Get test working with integration flipper

* Use `incrementer` for test instead

* Add tests to make sure that `LangError`s can be handled

* Get rid of references to the interation Flipper

Vulnerable code:
) -> Result<R>
    ) -> Result<R>
    /// 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.
        crate::invoke_contract_delegate(self).unwrap_or_else(|env_error| {
            panic!("Cross-contract call failed with {env_error:?}")
        })
    /// On failure this returns an [`ink::env::Error`][`crate::Error`] which can be
    pub fn try_invoke(&self) -> Result<R, crate::Error> {
    pub fn try_invoke(self) -> Result<(), Error> {
    /// 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.
    /// On failure this an [`ink::env::Error`][`crate::Error`] which can be handled by the
    /// caller.
    pub fn try_invoke(self) -> Result<R, Error> {
    ) -> Result<R>
    ) -> Result<R>
    ) -> Result<R>

Fixed code:
) -> Result<ink_primitives::MessageResult<R>>
    ) -> Result<ink_primitives::MessageResult<R>>
    /// 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.
        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:?}")
            })
    /// On failure this returns an outer [`ink::env::Error`][`crate::Error`] or inner
    /// [`ink::primitives::LangError`][`ink_primitives::LangError`], both of which can be
    pub fn try_invoke(&self) -> Result<ink_primitives::MessageResult<R>, crate::Error> {
    pub fn try_invoke(self) -> Result<ink_primitives::MessageResult<()>, Error> {
    /// 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.
    /// 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> {
    ) -> Result<ink_primitives::MessageResult<R>>
    ) -> Result<ink_primitives::MessageResult<R>>
    ) -> Result<ink_primitives::MessageResult<R>>
[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 = []
//! # 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(())
        }
    }
}