In this technical blog post, we detail the flaw found in the Dexter contract and the exploit used to “white-knight” the funds in those contracts.
The Dexter contract contains several entrypoints allowing users to perform various operations, such as adding and removing liquidity, or converting tokens to tez back and forth. The exact interface is given by the type of the contract’s parameter:
parameter (or (or (or (pair %approve (address :spender) (pair (nat :allowance) (nat :currentAllowance))) (pair %addLiquidity (pair (address :owner) (nat :minLqtMinted)) (pair (nat :maxTokensDeposited) (timestamp :deadline)))) (or (pair %removeLiquidity (pair (address :owner) (pair (address :to) (nat :lqtBurned))) (pair (mutez :minXtzWithdrawn) (pair (nat :minTokensWithdrawn) (timestamp :deadline)))) (or (pair %xtzToToken (address :to) (pair (nat :minTokensBought) (timestamp :deadline))) (pair %tokenToXtz (pair (address :owner) (address :to)) (pair (nat :tokensSold) (pair (mutez :minXtzBought) (timestamp :deadline))))))) (or (or (pair %tokenToToken (pair (address :outputDexterContract) (pair (nat :minTokensBought) (address :owner))) (pair (address :to) (pair (nat :tokensSold) (timestamp :deadline)))) (or (key_hash %updateTokenPool) (nat %updateTokenPoolInternal))) (or (pair %setBaker (option key_hash) bool) (or (address %setManager) (unit %default)))));
Of particular interest is the
tokenToXtz entrypoint, which allows the swapping of tokens for tez. In Tezos, entrypoints are represented as different cases of a single sum type representing the expected parameter. The type for
tokenToXtz is given as
(pair %tokenToXtz (pair (address :owner) (address :to)) (pair (nat :tokensSold) (pair (mutez :minXtzBought) (timestamp :deadline)))))))
This means that the entrypoint expects a record with the following fields
deadline: the time until which the transaction requested is valid
minXtzBought: the minimum amount of tez requested. If the tokens were to be swapped for fewer tez, the transaction be considered invalid, protecting the seller against excessive slippage
tokensSold: the number of tokens the seller desires to sell
to: the address where the tez proceeds are to be sent
owner: the owner of the tokens being sold
When the Dexter contract receives requests, it performs a series of computations to determine how many tez should be sold and, ultimately, emits two operations. One which sends tez to the “to” field, and the other, directed at the FA1.2 token contract, which request a transfer of tokens from the
owner to the Dexter contract itself. If any of these transactions fail, the whole transaction fails.
An FA1.2 transfer is valid if and only if the contract calling the FA1.2 contract owns the tokens being transferred, or if it is present in the allowance list for the owner of the tokens. This is a pattern found in ERC20 as well.
When Dexter users sell tokens to Dexter, they typically first make a temporary allowance in the FA1.2 contract to allow Dexter to access their funds.
The presence of an
owner field in
tokenToXtz raises two problems. First, it means a user is able to spend the token of anyone who has an open allowance to Dexter. While it is possible to recommend that allowances only be set temporarily and be reset to 0 after every interaction with the contract, this was not, in fact, done, and we found that several liquidity providers had small dangling allowances left after adding liquidity.
More problematic is the fact that the
owner field could simply be set to the Dexter contract’s address itself. Since the caller of an FA1.2 contract is always allowed to spend its own funds, this means that the Dexter contract could be instructed to dispense tez by sending itself its own tokens. Self-transfers are normal, and even required in the FA2 standard.
Dexter implements a constant formula market maker which attempts to maintain constant (before fees) the product of the quantity of the two assets it holds. By asking the Dexter contract to send itself its entire token balance, it’s possible to collect (before fees) roughly half of its tez balance.
The presence of an
owner field in
tokenToXtz was unnecessary, and its only safe value would be
SENDER, which renders it useless as a parameter.
It is important for authors of smart contracts to consider and test edge conditions of all sorts on all possible calls. This is especially true in situations where a contract can be told to transfer value to or from a third party. Creators of contracts should strongly consider avoiding such facilities entirely when they are not critical to the function of a contract. When providing such facilities, it is vital to consider whether edge cases might result in unexpected behavior.