balsoft/ligo
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Michelson: remove confusing MANAGER instruction

Browse Source
This commit is contained in:
Benjamin Canou 2018-06-19 17:31:16 +02:00
parent 3d602424d1
commit 1ccfe6aed9
8 changed files with 3 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
docs/tutorials/michelson_anti_patterns.rst
View File

@ -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.

20
docs/whitedoc/michelson.rst
View File

@ -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

1
src/proto_alpha/lib_protocol/src/alpha_context.mli
View File

@ -217,7 +217,6 @@ module Script : sig
| I_LSL
| I_LSR
| I_LT
| I_MANAGER
| I_MAP
| I_MEM
| I_MUL

4
src/proto_alpha/lib_protocol/src/michelson_v1_primitives.ml
View File

@ -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) ;

1
src/proto_alpha/lib_protocol/src/michelson_v1_primitives.mli
View File

@ -64,7 +64,6 @@ type prim =
| I_LSL
| I_LSR
| I_LT
| I_MANAGER
| I_MAP
| I_MEM
| I_MUL

12
src/proto_alpha/lib_protocol/src/script_interpreter.ml
View File

@ -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 ->

21
src/proto_alpha/lib_protocol/src/script_ir_translator.ml
View File

@ -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 ;

4
src/proto_alpha/lib_protocol/src/script_typed_ir.ml
View File

@ -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 :