TemplarSingle Chain Smart Contracts Security Review

The Supply Withdrawal process operates on a queue mechanism. First user creates a withdrawal request which is added to the queue. Then, separate and permissionless function (execute_next_supply_withdrawal_request) is processing those requests in order of their creation. There is a check in callback that verifies if the token transfer was successful. In case it wasn't, it essentially doesn't modify the queue so that next execute_next_supply_withdrawal_request call will try to process the same request. The assumption being that this transfer fails if Market contract doesn't have enough tokens to transfer. There is another possible case, that a malicious user can use to damage the protocol (and also create a ransom-like situation). NEP141 tokens allow users to unregister (withdraw their storage deposit). When that happens, their balance is completely removed from the token contract and transferring tokens to that user fails. A malicious user can:

Supply tokens Unregister from the token contract Create a supply withdrawal request

At that point, when withdrawal queue reaches that malicious user's request - the whole functionality stops functioning as no request can be processed at that time. This is a somewhat recoverable state, if token used for supply is following the default storage implementation, which allows anyone to register anyone else. However, that means someone else would need to cover storage fees for a malicious user in a token contract that this malicious user can then withdraw to himself by unregistering again, which can create a "vicious cycle" and essentially means malicious user would be requesting a ransom to allow supply withdrawal to function.

impl_market_external:execute_next_supply_withdrawal_request:market/src/impl_helper.rs

PromiseOrValue::Promise( self.configuration .borrow_asset .transfer( withdrawal_resolution.account_id.clone(), withdrawal_resolution.amount_to_account, ) .then( self_ext!(Self::GAS_AFTER_EXECUTE_NEXT_WITHDRAWAL)

.execute_next_supply_withdrawal_request_01_finalize(withdrawal_resolution), ), )}

impl_helper:execute_next_supply_withdrawal_request_01_finalize:market/src/impl_helper.rs

      PromiseResult::Failed => {
            // Withdrawal failed: unlock the queue so they can try again.
            // This occurs when the contract does not control enough of
            // the borrow asset to fulfill the withdrawal request. That is
            // to say, it has distributed all of the funds to current
            // borrows.

              env::log_str("The withdrawal request cannot be fulfilled at this time. Please try
     again later.");
             self.withdrawal_queue.unlock();
              if let Some(mut supply_position) =
                  self.supply_position_guard(withdrawal_resolution.account_id.clone())
              {
                  // This should not do very much since we also accumulate
                  // yield in the initial function call of execute_next_supply_withdrawal_request()
                  let proof = supply_position.accumulate_yield();
                  let mut amount = withdrawal_resolution.amount_to_account;
                  amount.join(withdrawal_resolution.amount_to_fees);
                  supply_position.record_deposit(proof, amount, env::block_timestamp_ms());
              }}

Impact:

Denial of Service condition on the supply withdrawal functionality, along with potential funds lost for users or protocol operators when trying to mitigate the Denial of Service.

Recommendation

Implement a mechanism that would utilize internal accounting to estimate whether a token transfer failed due to malicious user unregistering or due to missing token supply. Should the situation be determined as a result of malicious user's action, the Withdrawal Request should be removed from the queue.

Remediation - Fixed

The Templar team has fixed the issue.

The contract uses Pyth Oracle's list_ema_prices_no_older_than function to retrieve the prices. The functionalities using the price data will fail if the Oracle will not be able to provide prices, which will happen if the most recent available price is older than the configured threshold. It must be noted that currently there is a bot service running on mainnet that seems to be updating the prices, however, that might not always be the case. Should the bot be disabled or otherwise stop working, the core functionalities of the contract will not be possible to execute successfuly.

Impact:

Possible Denial of Service leading to a potential economic repercussions due to inability to execute actions related to the price data.

Recommendation

It is recommended to implement a fail-safe mechanism that would be able to use Pyth's update_price_feeds endpoint in order to update the prices manually, should the bot stop being reliable.

Remediation - Acknowledged

The Templar team has acknowledged the issue by stating that there are already measures implemented preventing that scenario.

It was observed that the contract does not implement any mechanism that would prevent users from self-liquidating. Such an action does not directly damage the protocol and requires a liquidable user to win a race with other liquidators. Should this situation happen, this scenario translates to a repayment with a discount rather than a liquidation.

It must be noted that ultimately this scenario is not completely preventable, as users are free to operate using many different AccountIds.

Impact:

Cheaper loan repayment.

Recommendation

It is recommended to add a check that would at least prevent a self-liquidation.

Remediation - Acknowledged

The Templar team has acknowledged the issue but it won't be fixed.