Disclaimer: This issue is already fixed in the latest internal Flex development (not in master branch yet), and it is opened solely to manifest our work results in bug-finding.

https://github.com/tonlabs/flex/blob/5a83080941cb5d82470ec5bd0070ebb36ce6a7ec/flex/PriceXchg.cpp#L187

In PriceXchg, when the deal completes with buy_out_of_tons, the contract has to return the tip3 ownership of the both sell token and the buy token. It does this in case the coins left is enough to cover messaging expenses. In the aforementioned line, for the sell case, the condition incorrectly refers to the sell.account value instead of buy.account.

Before executing the order, Price contract searches for the most recent active order to process, filtering out all inactive orders.
It does this by traversing the order queue here:
https://github.com/tonlabs/flex/blob/3aaa20eb73e7498d08d39191d693f1efa7d016eb/flex/Price.cpp#L84

The loop is not bounded by any means, and, in case of big queues, such processing may lead to out_of_gas exception.
From this point, the Price contract will no longer be able to process orders.

Here's a typo: tonos-cli debot --url net.ton.dev debot fetch 0:81c12c2f4514124536aafea59db7df0262d3af877b4477afe6514bbc5bc9f317

Should be: tonos-cli --url net.ton.dev debot fetch 0:81c12c2f4514124536aafea59db7df0262d3af877b4477afe6514bbc5bc9f317

https://github.com/tonlabs/flex#tip3-debot
https://github.com/tonlabs/flex#flex-debot

In the PriceXchg::cancelBuy(), the function cancel_order_impl() is triggered to traverse the orders queue and cancel each,
for the given client address. It sends back the ret_val coins to client that put the order. Right now, for each canceled order, the ret_val is calculated as plus_val - minus_val,
where plus_val == ord.account + (is_first ? incoming_val : 0) , and
minus_val == process_queue + return_ownership.

https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/PriceXchg.cpp#L282
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/PriceXchg.cpp#L285
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/PriceXchg.cpp#L287

The minus_val here includes the queue processing fee for each order canceled, while the correct logic is to charge this fee only once, for all orders. The correct way of doing it is presented in the Price::cancel_order_impl()

https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/Price.cpp#L257

In Flex, orders of opposite direction get matched in the dealer::process_queue() function.

This function contains the while loop that traverses the order queues of opposite direction, matching orders with each other.
It is quite message-intensive function: for example, it generates at least 3 new messages for each partially matched order
(see dealer::make_deal), and on each loop iteration, it generates a message for every expired order found in the queue
(see dealer::extract_active_order).

The proposition of this issue is that in some circumstances, the process_queue() function may generate more messages
than allowed by the node, i.e. MAX_ACTIONS = 255.
(see https://github.com/tonlabs/ton-labs-executor/blob/e20b16b4d92cee8261e96916b118c344483e0013/src/transaction_executor.rs#L488)
If this happens, most probably, the Price smart-contract will not be able to match orders of this price level any further :
all future calls will run into the same transaction abort, leaving all the funds locked and this Price level locked.

Before doing the formal analysis, we thought that the number of generated messages is bounded by at least the deals_limit_
constant, but, it turns out, the loop iteration counter may go well beyond that value. And the reason is that the break statement is located under conditional that might not hold, leading to another loop iterations:
https://github.com/tonlabs/flex/blob/3aaa20eb73e7498d08d39191d693f1efa7d016eb/flex/Price.cpp#L139

It is proposed to introduce some definitional bound for the number of iterations this loop may process, hence protecting
the action queue from possible overflow / deadlock condition.

The same holds for PriceXchg::process_queue().

In the PriceXchg.cpp, when calculating the cost of a deal, the function __builtin_tvm_muldivr(a,b,c) is used.
This function throws exception in case of c == 0. This value corresponds to the denominator of the exchange.

Right now, there is no check that prevents this code from executing in case of denominator = 0.
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/PriceXchg.cpp#L39

From business logic point of view, this scenario corresponds to tokens "give away", we do not ask anything
for our tokens. Such scenario is not very likely, however, possible. For example, during some airdrops.

We advice to either fix this calc statement, or add a check preventing the system from deploying such exchanges completely.

In the dealer::make_deal(), the tokens amount is decreased unconditionally:
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/Price.cpp#L47
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/Price.cpp#L48

If the the function returns sell_out_of_tons or buy_out_of_tons, the process_queue() function will use the
sell.original_amount - sell.amount and/or buy.original_amount - buy_amount as the actual processed tokens value in the following lines:
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/Price.cpp#L151
https://github.com/tonlabs/flex/blob/ededddc1ea37f7e40297ed5aafe06d5e4c1ff870/flex/Price.cpp#L165

This is imprecise because the actual amount of processed tokens is different in this case.

In the Price.cpp, dealer::make_deal(), the following statements appear:
https://github.com/tonlabs/flex/blob/faa9d5bc41ea10325290e9d3fbf75fea8035d5bd/flex/Price.cpp#L49
https://github.com/tonlabs/flex/blob/faa9d5bc41ea10325290e9d3fbf75fea8035d5bd/flex/Price.cpp#L54

In case the product deal_amount * price_ overflows the uint128 type, the cost becomes an empty option<>.
In this case, dereferencing it in line 54 results in undefined behavior, as stated in the C++ reference:
https://en.cppreference.com/w/cpp/utility/optional/operator*
meaning that cost may become an arbitrary value.

Right now, the issue is non relevant, because make_dealer is used within the context of process_queue_impl, after the checks are made, but if the dealer is used in another context with checks forgotten, it will result in interesting surprises.

Note that in the PriceXchg.cpp, the corresponding check is present:
https://github.com/tonlabs/flex/blob/faa9d5bc41ea10325290e9d3fbf75fea8035d5bd/flex/PriceXchg.cpp#L56
https://github.com/tonlabs/flex/blob/faa9d5bc41ea10325290e9d3fbf75fea8035d5bd/flex/PriceXchg.cpp#L59

After the constructor call, the RootTokenContract to be fully initialized, the method setWalletCode has to be called.
This method sets the code for TONTokenWallet contracts being deployed on request.

The function calc_wallet_init relies on the wallet_code_.get() value, and uses it without any prior checks.
In case the wallet_code_ field is not initialized, the call will throw exception. This is nothing special, but in the deployWallet() case, it may happen after the tvm.accept() call. In this case, excessive coins loss may happen due to charging by all network validators.

https://github.com/tonlabs/flex/blob/6d4279d94dfa91b0e2c8670a78282e223fef2025/tokens-fungible/RootTokenContract.cpp#L91

It is proposed to add require(hasWalletCode(), error_code::wallet_code_not_initialized) checks into both deployWallet and deployEmptyWallet methods.

In the Flex, TONTokenWallet.cpp, the token lending functionality is introduced. It allows a second party to use token on behalf of the owner.

Currently, all lending permissions are stored in the lend_ownership_ map. This map is iterated each time the check_owner() function gets called. This function gets called from many places, including the blocks that are executed before tvm.accept(), and, hence, especially vulnerable to out-of-gas error. In case the Token receives a lot of lending requests, the lend_ownership_ map may become big enough to cause out-of-gas exception, possibly leading to the token deadlock.

This is were iteration over map takes place:
https://github.com/tonlabs/flex/blob/6d4279d94dfa91b0e2c8670a78282e223fef2025/tokens-fungible/TONTokenWallet.cpp#L561

This is an example of a call before tvm.accept():
https://github.com/tonlabs/flex/blob/6d4279d94dfa91b0e2c8670a78282e223fef2025/tokens-fungible/TONTokenWallet.cpp#L450

We propose to put some strict upper bound on the allowed number of lending records for a token.