From f62998fe963a2633d743ff1262586b62c9c2c76a Mon Sep 17 00:00:00 2001 From: Pierrick Couderc Date: Thu, 28 Jul 2022 16:03:44 +0200 Subject: [PATCH 1/3] Client: disable origination of contracts containing timelock instructions --- .../lib_client/client_proto_context.ml | 21 +++- tezt/tests/main.ml | 1 + tezt/tests/timelock.ml | 98 +++++++++++++++++++ 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 tezt/tests/timelock.ml diff --git a/src/proto_alpha/lib_client/client_proto_context.ml b/src/proto_alpha/lib_client/client_proto_context.ml index 3f684db13a66..5c3d6e0a74d3 100644 --- a/src/proto_alpha/lib_client/client_proto_context.ml +++ b/src/proto_alpha/lib_client/client_proto_context.ml @@ -386,8 +386,24 @@ let save_contract ~force cctxt alias_name contract = RawContractAlias.add ~force cctxt alias_name contract >>=? fun () -> message_added_contract cctxt alias_name >>= fun () -> return_unit -let build_origination_operation ?fee ?gas_limit ?storage_limit ~initial_storage - ~code ~delegate ~balance () = +let check_for_timelock code = + let open Tezos_micheline.Micheline in + let rec has_timelock_opcodes = function + | Prim (_, (Script.T_chest | T_chest_key | I_OPEN_CHEST), _, _) -> true + | Seq (_, exprs) | Prim (_, _, exprs, _) -> + List.exists has_timelock_opcodes exprs + | Int _ | String _ | Bytes _ -> false + in + has_timelock_opcodes (root code) + +let build_origination_operation ?(allow_timelock = false) ?fee ?gas_limit + ?storage_limit ~initial_storage ~code ~delegate ~balance () = + (if (not allow_timelock) && check_for_timelock code then + failwith + "Origination of contracts containing time lock related instructions is \ + disabled in the client because of a vulnerability." + else return_unit) + >>=? fun () -> (* With the change of making implicit accounts delegatable, the following 3 arguments are being defaulted before they can be safely removed. *) Lwt.return (Michelson_v1_parser.parse_expression initial_storage) @@ -409,6 +425,7 @@ let originate_contract (cctxt : #full) ~chain ~block ?confirmations ?dry_run ?verbose_signing ?branch ?fee ?gas_limit ?storage_limit ~delegate ~initial_storage ~balance ~source ~src_pk ~src_sk ~code ~fee_parameter () = build_origination_operation + ~allow_timelock:false ?fee ?gas_limit ?storage_limit diff --git a/tezt/tests/main.ml b/tezt/tests/main.ml index 3ac2b72a3f41..157ab1c46241 100644 --- a/tezt/tests/main.ml +++ b/tezt/tests/main.ml @@ -131,6 +131,7 @@ let register_J_plus_tests () = Multinode_snapshot.register ~protocols:[Alpha] ; Run_script.register ~protocols:[Alpha] ; Sapling.register ~protocols:[Alpha] ; + Timelock.register ~protocols ; Tx_rollup.register ~protocols ; Tx_rollup_node.register ~protocols ; Views.register [Alpha] diff --git a/tezt/tests/timelock.ml b/tezt/tests/timelock.ml new file mode 100644 index 000000000000..98ba13f78bff --- /dev/null +++ b/tezt/tests/timelock.ml @@ -0,0 +1,98 @@ +(*****************************************************************************) +(* *) +(* Open Source License *) +(* Copyright (c) 2022 Nomadic Labs *) +(* *) +(* Permission is hereby granted, free of charge, to any person obtaining a *) +(* copy of this software and associated documentation files (the "Software"),*) +(* to deal in the Software without restriction, including without limitation *) +(* the rights to use, copy, modify, merge, publish, distribute, sublicense, *) +(* and/or sell copies of the Software, and to permit persons to whom the *) +(* Software is furnished to do so, subject to the following conditions: *) +(* *) +(* The above copyright notice and this permission notice shall be included *) +(* in all copies or substantial portions of the Software. *) +(* *) +(* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR*) +(* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, *) +(* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL *) +(* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER*) +(* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING *) +(* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER *) +(* DEALINGS IN THE SOFTWARE. *) +(* *) +(*****************************************************************************) + +(* Testing + ------- + Component: Client commands + Invocation: dune exec tezt/tests/main.exe -- --file timelock.ml + Subject: Tests checking contracts with timelock cannot be originated +*) + +(* Script extracted from + `src/proto_alpha/lib_protocol/test/integration/michelson/contracts/timelock.tz` +*) +let script = + {| +storage (bytes); +parameter (pair (chest_key) (chest)); +code { + UNPAIR; + DIP {DROP}; + UNPAIR; + DIIP {PUSH nat 1000}; + OPEN_CHEST; + IF_LEFT + { # successful case + NIL operation; + PAIR ; + } + { + IF + { # first type of failure + PUSH bytes 0x01; + NIL operation; + PAIR; + } + { # second type of failure + PUSH bytes 0x00; + NIL operation; + PAIR; + } + } + } + |} + +let test_contract_not_originable ~protocol () = + let* client = Client.init_mockup ~protocol () in + let result = + Client.spawn_originate_contract + ~alias:Constant.bootstrap1.Account.alias + ~amount:Tez.zero + ~src:Constant.bootstrap1.alias + ~prg:script + ~init:"0xaa" + ~burn_cap:(Tez.of_int 1) + client + in + let msg = + rex + "Origination of contracts containing time lock related instructions is \ + disabled in the client because of a vulnerability." + in + Process.check_error ~exit_code:1 ~msg result + +let register ~protocols = + List.iter + (fun (title, test_function) -> + Protocol.register_test + ~__FILE__ + ~title + ~tags:["client"; "michelson"; "timelock"] + (fun protocol -> test_function ~protocol ()) + protocols) + [ + ( "Check a contract containing timelock operations is forbidden", + test_contract_not_originable ); + ] -- GitLab From 8b109844b8bbd7851d1caab817332324cc4b9eb2 Mon Sep 17 00:00:00 2001 From: Pierrick Couderc Date: Thu, 28 Jul 2022 16:04:41 +0200 Subject: [PATCH 2/3] Client: backport disabled timelock in 013 and 014 clients --- .../lib_client/client_proto_context.ml | 21 +++++++++++++++++-- .../lib_client/client_proto_context.ml | 21 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/proto_013_PtJakart/lib_client/client_proto_context.ml b/src/proto_013_PtJakart/lib_client/client_proto_context.ml index 4d393a867417..c070dfe1c739 100644 --- a/src/proto_013_PtJakart/lib_client/client_proto_context.ml +++ b/src/proto_013_PtJakart/lib_client/client_proto_context.ml @@ -352,8 +352,24 @@ let save_contract ~force cctxt alias_name contract = RawContractAlias.add ~force cctxt alias_name contract >>=? fun () -> message_added_contract cctxt alias_name >>= fun () -> return_unit -let build_origination_operation ?fee ?gas_limit ?storage_limit ~initial_storage - ~code ~delegate ~balance () = +let check_for_timelock code = + let open Tezos_micheline.Micheline in + let rec has_timelock_opcodes = function + | Prim (_, (Script.T_chest | T_chest_key | I_OPEN_CHEST), _, _) -> true + | Seq (_, exprs) | Prim (_, _, exprs, _) -> + List.exists has_timelock_opcodes exprs + | Int _ | String _ | Bytes _ -> false + in + has_timelock_opcodes (root code) + +let build_origination_operation ?(allow_timelock = false) ?fee ?gas_limit + ?storage_limit ~initial_storage ~code ~delegate ~balance () = + (if (not allow_timelock) && check_for_timelock code then + failwith + "Origination of contracts containing time lock related instructions is \ + disabled in the client because of a vulnerability." + else return_unit) + >>=? fun () -> (* With the change of making implicit accounts delegatable, the following 3 arguments are being defaulted before they can be safely removed. *) Lwt.return (Michelson_v1_parser.parse_expression initial_storage) @@ -375,6 +391,7 @@ let originate_contract (cctxt : #full) ~chain ~block ?confirmations ?dry_run ?verbose_signing ?branch ?fee ?gas_limit ?storage_limit ~delegate ~initial_storage ~balance ~source ~src_pk ~src_sk ~code ~fee_parameter () = build_origination_operation + ~allow_timelock:false ?fee ?gas_limit ?storage_limit diff --git a/src/proto_014_PtKathma/lib_client/client_proto_context.ml b/src/proto_014_PtKathma/lib_client/client_proto_context.ml index 554d7d86ad2b..13aae0d278ed 100644 --- a/src/proto_014_PtKathma/lib_client/client_proto_context.ml +++ b/src/proto_014_PtKathma/lib_client/client_proto_context.ml @@ -386,8 +386,24 @@ let save_contract ~force cctxt alias_name contract = RawContractAlias.add ~force cctxt alias_name contract >>=? fun () -> message_added_contract cctxt alias_name >>= fun () -> return_unit -let build_origination_operation ?fee ?gas_limit ?storage_limit ~initial_storage - ~code ~delegate ~balance () = +let check_for_timelock code = + let open Tezos_micheline.Micheline in + let rec has_timelock_opcodes = function + | Prim (_, (Script.T_chest | T_chest_key | I_OPEN_CHEST), _, _) -> true + | Seq (_, exprs) | Prim (_, _, exprs, _) -> + List.exists has_timelock_opcodes exprs + | Int _ | String _ | Bytes _ -> false + in + has_timelock_opcodes (root code) + +let build_origination_operation ?(allow_timelock = false) ?fee ?gas_limit + ?storage_limit ~initial_storage ~code ~delegate ~balance () = + (if (not allow_timelock) && check_for_timelock code then + failwith + "Origination of contracts containing time lock related instructions is \ + disabled in the client because of a vulnerability." + else return_unit) + >>=? fun () -> (* With the change of making implicit accounts delegatable, the following 3 arguments are being defaulted before they can be safely removed. *) Lwt.return (Michelson_v1_parser.parse_expression initial_storage) @@ -409,6 +425,7 @@ let originate_contract (cctxt : #full) ~chain ~block ?confirmations ?dry_run ?verbose_signing ?branch ?fee ?gas_limit ?storage_limit ~delegate ~initial_storage ~balance ~source ~src_pk ~src_sk ~code ~fee_parameter () = build_origination_operation + ~allow_timelock:false ?fee ?gas_limit ?storage_limit -- GitLab From c82d4e1c385fb82a721a288d9535bd2f723443f9 Mon Sep 17 00:00:00 2001 From: Pierrick Couderc Date: Thu, 28 Jul 2022 16:29:15 +0200 Subject: [PATCH 3/3] Client: update Changes --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index e56ea6a1c533..f8c1f42b7b1e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -104,6 +104,8 @@ Client - Added `--ignore-case` option to the `tezos-client gen vanity keys` command to allow case-insensitive search for the given pattern. +- Disabled origination of contracts with timelock instructions. + Accuser ------- -- GitLab