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.
Background
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 validminXtzBought
: 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 slippagetokensSold
: the number of tokens the seller desires to sellto
: the address where the tez proceeds are to be sentowner
: 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.
Flaw
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.
Conclusion
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.