Omniscia fetchai Audit
ALPTotalDebts Manual Review Findings
ALPTotalDebts Manual Review Findings
ALP-01M: Privileged Re-Entrancy Attack
| Type | Severity | Location |
|---|---|---|
| Language Specific | Medium | ALPTotalDebts.sol:L206-L240 |
Description:
The way the transfer of the ACT amounts is performed allows the trustee to re-enter the contract at an improper state, breaking the Checks-Effects-Interactions pattern.
Example:
206function transferLoan(207 address fromAccount,208 uint256 fromACTId,209 address toAccount,210 uint256 toACTId,211 bool swapAssetData212) external whenNotPaused() {213 // This needs to be done first so the individual functions can do validation tests214 // before transferring the actual ACT215 if (swapAssetData) {216 tokenizerContract.swapACT(fromACTId, toACTId);217 }218
219 // Swap the records of the loan data220 uint256 lpACTAmount =221 lendingPoolContract.transferLoanData(222 fromAccount,223 fromACTId,224 toAccount,225 toACTId,226 msg.sender227 );228 uint256 llACTAmount =229 loanLiquidatorContract.transferLoanData(230 fromAccount,231 fromACTId,232 toAccount,233 toACTId,234 msg.sender235 );236
237 // transfer the actual ACT tokens in and out238 lendingPoolContract.transferLoanACT(fromACTId, toACTId, lpACTAmount, msg.sender);239 loanLiquidatorContract.transferLoanACT(fromACTId, toACTId, llACTAmount, msg.sender);240}Recommendation:
We advise this trait to be carefully assessed as it can lead to high-severity consequences due to the loan accounting system being frozen at an improper state.
Alleviation:
The Atomix team restructured the way the transfers of the NFTs are performed by ensuring that only the last "leg" of the transfers is performed to a potentially re-entrant wallet, thereby conforming to the Checks-Effects-Interactions pattern and preventing any form of re-entrancy to occur in an unwanted state.
ALP-02M: Improper Handling of Zero Values
| Type | Severity | Location |
|---|---|---|
| Logical Fault | Minor | ALPTotalDebts.sol:L228-L235, L239 |
Description:
The transferLoan function should gracefully handle zero-value ACT amounts in the loan liquidator as the loan liquidator is not guaranteed to hold a non-zero amount of ACT and can cause certain transfer functions to fail.
Example:
220uint256 lpACTAmount =221 lendingPoolContract.transferLoanData(222 fromAccount,223 fromACTId,224 toAccount,225 toACTId,226 msg.sender227 );228uint256 llACTAmount =229 loanLiquidatorContract.transferLoanData(230 fromAccount,231 fromACTId,232 toAccount,233 toACTId,234 msg.sender235 );236
237// transfer the actual ACT tokens in and out238lendingPoolContract.transferLoanACT(fromACTId, toACTId, lpACTAmount, msg.sender);239loanLiquidatorContract.transferLoanACT(fromACTId, toACTId, llACTAmount, msg.sender);Recommendation:
We advise the ACT of the loan liquidator to be opportunistically transferred by introducing a simple if (llACTAmount > 0) check preceding the loanLiquidatorContract.transferLoanACT call.
Alleviation:
Such a check has been introduced as suggested in the actual ERC1155 safeTransferFrom function rather than the code of the contract itself thus alleviating this exhibit.
ALP-03M: Improperly Handled Edge Case
| Type | Severity | Location |
|---|---|---|
| Logical Fault | Minor | ALPTotalDebts.sol:L107 |
Description:
The repay function will evaluate that a debt exists in the loan liquidator after the debt in lending pool is cleared and if it exists, pass in the amountRemaining to be repaid.
Example:
102if (amountRemaining > 0) {103 // If we have some left to repay, but there is nothing to repay, then error104 uint256 accountLlDebt = loanLiquidatorContract.getBorrowerDebt(account, actId);105 require(accountLlDebt > 0, "ALPTotalDebts: Exceeds the account debt");106
107 loanLiquidatorContract.repay(msg.sender, account, actId, amountRemaining);108}Recommendation:
We advise the Math.min(amountRemaining, accountLlDebt) to be passed in to loanLiquidatorContract.repay instead to ensure that the function will successfully execute as it will fail otherwise.
Alleviation:
The Atomix team has stated that this is expected behaviours as should a user desire to repay the full amount they should invoke the repayAll function. As a result, we consider this exhibit null.