From 1ccfe6aed9e13b32b3972153f19e3e8f32c561b6 Mon Sep 17 00:00:00 2001 From: Benjamin Canou Date: Tue, 19 Jun 2018 17:31:16 +0200 Subject: [PATCH] Michelson: remove confusing MANAGER instruction --- docs/tutorials/michelson_anti_patterns.rst | 43 ------------------- docs/whitedoc/michelson.rst | 20 --------- .../lib_protocol/src/alpha_context.mli | 1 - .../src/michelson_v1_primitives.ml | 4 -- .../src/michelson_v1_primitives.mli | 1 - .../lib_protocol/src/script_interpreter.ml | 12 ------ .../lib_protocol/src/script_ir_translator.ml | 21 ++------- .../lib_protocol/src/script_typed_ir.ml | 4 -- 8 files changed, 3 insertions(+), 103 deletions(-) diff --git a/docs/tutorials/michelson_anti_patterns.rst b/docs/tutorials/michelson_anti_patterns.rst index 8a17fdee3..bf6ad897e 100644 --- a/docs/tutorials/michelson_anti_patterns.rst +++ b/docs/tutorials/michelson_anti_patterns.rst @@ -201,46 +201,3 @@ Alternatives/Solutions ~~~~~~~~~~~~~~~~~~~~~~ - Do not store funds in spendable contracts that you do not control. - -Do not use ``SENDER ; MANAGER`` for authentication -------------------------------------------------- - -Each originated account has a manager. The manager may change the -delegate of the account or issue transfers from it (depending on the -'delegatable' and 'spendable' flags). - -When an account is originated, the originator can set the manager to -any key hash. The originator also sets the code for contracts, and so -could immediately transfer to the originated contract, causing it in -turn to transfer to your contract. This means **an attacker may -arrange for the manager of the source to be any arbitrary key hash**, -without ever having access to the corresponding key. - -.. _possible-issues-6: - -Possible Issues -~~~~~~~~~~~~~~~ - -- An attacker may trivially subvert your authentication code by - originating a contract with manager set to the key hash you are - looking for. -- A user may (voluntarily) manage contracts which execute transfers - without their approval. Contracts should not (and, given the above, - must not) be authorized to act on behalf of their managers. - -.. _alternativessolutions-6: - -Alternatives/Solutions -~~~~~~~~~~~~~~~~~~~~~~ - -- Directly compare the ``SENDER`` address to the expected sender, - ideally an implicit account associated with some trusted key - hash. If you have a trusted ``key_hash`` rather than an - ``address``, you can obtain an address to compare against the - source using ``IMPLICIT_ACCOUNT ; ADDRESS``. If the expected sender - is a contract, you may want to understand its code -- it could - allow anyone to cause it to transfer to your contract. -- For more flexibility, require authorization via a signature of data - describing the operation you will perform, and use - ``CHECK_SIGNATURE`` to verify it. See above regarding replay - attacks. diff --git a/docs/whitedoc/michelson.rst b/docs/whitedoc/michelson.rst index c4d87d65a..de941935f 100644 --- a/docs/whitedoc/michelson.rst +++ b/docs/whitedoc/michelson.rst @@ -1295,17 +1295,6 @@ for under/overflows. Operations on contracts ~~~~~~~~~~~~~~~~~~~~~~~ -- ``MANAGER``: Access the manager of a contract. - -:: - - :: address : 'S -> key_hash option : 'S - :: contract 'p : 'S -> key_hash : 'S - -Note that the originator of an account/contract may set the manager to -**any** key hash. Thus, ``SENDER; MANAGER`` is not appropriate for -authentication. - - ``CREATE_CONTRACT``: Forge a contract creation operation. :: @@ -1405,9 +1394,6 @@ contract, unit for an account. internal transaction. It may be the ``SOURCE``, but may also not if the source sent an order to an intermediate smart contract, which then called the current contract. - To make sure that ``SENDER`` is the ``SOURCE``, either - compare them, or make sure that ``SENDER`` is the implicit - account of its ``MANAGER``. :: @@ -2036,7 +2022,6 @@ The instructions which accept at most one variable annotation are: GE ADDRESS CONTRACT - MANAGER SET_DELEGATE IMPLICIT_ACCOUNT NOW @@ -2304,10 +2289,6 @@ A similar mechanism is used for context dependent instructions: CONTRACT 'p :: @a address : 'S -> @a.contract contract 'p : 'S - MANAGER - :: @a address : 'S -> @a.manager key_hash option : 'S - :: @c contract 'p : 'S -> @c.manager key_hash : 'S - BALANCE :: 'S -> @balance tez : 'S SOURCE :: 'S -> @source address : 'S @@ -2921,7 +2902,6 @@ XII - Full grammar | LE | GE | INT - | MANAGER | SELF | TRANSFER_TOKENS | SET_DELEGATE diff --git a/src/proto_alpha/lib_protocol/src/alpha_context.mli b/src/proto_alpha/lib_protocol/src/alpha_context.mli index 6aa8c6b12..1d9523367 100644 --- a/src/proto_alpha/lib_protocol/src/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/src/alpha_context.mli @@ -217,7 +217,6 @@ module Script : sig | I_LSL | I_LSR | I_LT - | I_MANAGER | I_MAP | I_MEM | I_MUL diff --git a/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.ml b/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.ml index 17a080202..6fc13dfab 100644 --- a/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.ml +++ b/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.ml @@ -66,7 +66,6 @@ type prim = | I_LSL | I_LSR | I_LT - | I_MANAGER | I_MAP | I_MEM | I_MUL @@ -194,7 +193,6 @@ let string_of_prim = function | I_LSL -> "LSL" | I_LSR -> "LSR" | I_LT -> "LT" - | I_MANAGER -> "MANAGER" | I_MAP -> "MAP" | I_MEM -> "MEM" | I_MUL -> "MUL" @@ -303,7 +301,6 @@ let prim_of_string = function | "LSL" -> ok I_LSL | "LSR" -> ok I_LSR | "LT" -> ok I_LT - | "MANAGER" -> ok I_MANAGER | "MAP" -> ok I_MAP | "MEM" -> ok I_MEM | "MUL" -> ok I_MUL @@ -457,7 +454,6 @@ let prim_encoding = ("LSL", I_LSL) ; ("LSR", I_LSR) ; ("LT", I_LT) ; - ("MANAGER", I_MANAGER) ; ("MAP", I_MAP) ; ("MEM", I_MEM) ; ("MUL", I_MUL) ; diff --git a/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.mli b/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.mli index 1fe42843c..5aa0c5445 100644 --- a/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.mli +++ b/src/proto_alpha/lib_protocol/src/michelson_v1_primitives.mli @@ -64,7 +64,6 @@ type prim = | I_LSL | I_LSR | I_LT - | I_MANAGER | I_MAP | I_MEM | I_MUL diff --git a/src/proto_alpha/lib_protocol/src/script_interpreter.ml b/src/proto_alpha/lib_protocol/src/script_interpreter.ml index 04ad497e4..73894f1f9 100644 --- a/src/proto_alpha/lib_protocol/src/script_interpreter.ml +++ b/src/proto_alpha/lib_protocol/src/script_interpreter.ml @@ -610,18 +610,6 @@ let rec interp logged_return (Item (Some contract, rest), ctxt) else logged_return (Item (None, rest), ctxt) - | Manager, Item ((_, contract), rest) -> - Lwt.return (Gas.consume ctxt Interp_costs.manager) >>=? fun ctxt -> - Contract.get_manager ctxt contract >>=? fun manager -> - logged_return (Item (manager, rest), ctxt) - | Address_manager, Item (contract, rest) -> - Lwt.return (Gas.consume ctxt Interp_costs.manager) >>=? fun ctxt -> - Contract.exists ctxt contract >>=? fun exists -> - if exists then - Contract.get_manager ctxt contract >>=? fun manager -> - logged_return (Item (Some manager, rest), ctxt) - else - logged_return (Item (None, rest), ctxt) | Transfer_tokens, Item (p, Item (amount, Item ((tp, destination), rest))) -> Lwt.return (Gas.consume ctxt Interp_costs.transfer) >>=? fun ctxt -> diff --git a/src/proto_alpha/lib_protocol/src/script_ir_translator.ml b/src/proto_alpha/lib_protocol/src/script_ir_translator.ml index 1160af0ca..c2eccf495 100644 --- a/src/proto_alpha/lib_protocol/src/script_ir_translator.ml +++ b/src/proto_alpha/lib_protocol/src/script_ir_translator.ml @@ -193,8 +193,6 @@ let number_of_generated_growing_types : type b a. (b, a) instr -> int = function | Ge -> 0 | Address -> 0 | Contract _ -> 1 - | Manager -> 0 - | Address_manager -> 0 | Transfer_tokens -> 1 | Create_account -> 0 | Implicit_account -> 0 @@ -278,7 +276,6 @@ let namespace = function | I_LSL | I_LSR | I_LT - | I_MANAGER | I_MAP | I_MEM | I_MUL @@ -2264,18 +2261,6 @@ and parse_instr >>=? fun annot -> typed ctxt loc (Contract t) (Item_t (Option_t ((Contract_t (t, None), None), None, None), rest, annot)) - | Prim (loc, I_MANAGER, [], annot), - Item_t (Contract_t _, rest, contract_annot) -> - parse_var_annot loc annot ~default:(gen_access_annot contract_annot default_manager_annot) - >>=? fun annot -> - typed ctxt loc Manager - (Item_t (Key_hash_t None, rest, annot)) - | Prim (loc, I_MANAGER, [], annot), - Item_t (Address_t _, rest, addr_annot) -> - parse_var_annot loc annot ~default:(gen_access_annot addr_annot default_manager_annot) - >>=? fun annot -> - typed ctxt loc Address_manager - (Item_t (Option_t ((Key_hash_t None, None), None, None), rest, annot)) | Prim (loc, I_TRANSFER_TOKENS, [], annot), Item_t (p, Item_t (Mutez_t _, Item_t @@ -2408,7 +2393,7 @@ and parse_instr | I_ABS | I_NEG | I_LSL | I_LSR | I_COMPARE | I_EQ | I_NEQ | I_LT | I_GT | I_LE | I_GE - | I_MANAGER | I_TRANSFER_TOKENS | I_CREATE_ACCOUNT + | I_TRANSFER_TOKENS | I_CREATE_ACCOUNT | I_CREATE_CONTRACT | I_SET_DELEGATE | I_NOW | I_IMPLICIT_ACCOUNT | I_AMOUNT | I_BALANCE | I_CHECK_SIGNATURE | I_HASH_KEY | I_SOURCE | I_SENDER @@ -2453,7 +2438,7 @@ and parse_instr fail (Bad_stack (loc, I_TRANSFER_TOKENS, 4, stack)) | Prim (loc, (I_DROP | I_DUP | I_CAR | I_CDR | I_SOME | I_H | I_DIP | I_IF_NONE | I_LEFT | I_RIGHT | I_IF_LEFT | I_IF - | I_LOOP | I_IF_CONS | I_MANAGER | I_IMPLICIT_ACCOUNT + | I_LOOP | I_IF_CONS | I_IMPLICIT_ACCOUNT | I_NEG | I_ABS | I_INT | I_NOT | I_EQ | I_NEQ | I_LT | I_GT | I_LE | I_GE as name), _, _), stack -> @@ -2478,7 +2463,7 @@ and parse_instr I_ABS ; I_INT; I_NEG ; I_LSL ; I_LSR ; I_COMPARE ; I_EQ ; I_NEQ ; I_LT ; I_GT ; I_LE ; I_GE ; - I_MANAGER ; I_TRANSFER_TOKENS ; I_CREATE_ACCOUNT ; + I_TRANSFER_TOKENS ; I_CREATE_ACCOUNT ; I_CREATE_CONTRACT ; I_NOW ; I_AMOUNT ; I_BALANCE ; I_IMPLICIT_ACCOUNT ; I_CHECK_SIGNATURE ; I_H ; I_HASH_KEY ; I_STEPS_TO_QUOTA ; diff --git a/src/proto_alpha/lib_protocol/src/script_typed_ir.ml b/src/proto_alpha/lib_protocol/src/script_typed_ir.ml index efa22808e..a48e6aac1 100644 --- a/src/proto_alpha/lib_protocol/src/script_typed_ir.ml +++ b/src/proto_alpha/lib_protocol/src/script_typed_ir.ml @@ -318,10 +318,6 @@ and ('bef, 'aft) instr = (_ typed_contract * 'rest, Contract.t * 'rest) instr | Contract : 'p ty -> (Contract.t * 'rest, 'p typed_contract option * 'rest) instr - | Manager : - ('arg typed_contract * 'rest, public_key_hash * 'rest) instr - | Address_manager : - (Contract.t * 'rest, public_key_hash option * 'rest) instr | Transfer_tokens : ('arg * (Tez.t * ('arg typed_contract * 'rest)), packed_internal_operation * 'rest) instr | Create_account :