# BBB:Backed Protocol contest ## Title ###  [H-2] どのlenderもbuyoutしようとすると、currentLoanOwnerがloanInfoを操作することができる脆弱性 ### Category Re-entrancy ### Conditions • 攻撃者がlend()の呼び出し、貸し手(被害者)が攻撃者のローンを買い取ろうとした場合。 ### Hacking details ```solidity ERC20(loanAssetContractAddress).safeTransfer( currentLoanOwner, accumulatedInterest + previousLoanAmount ); ``` 攻撃者が既に lend() を呼び出して貸し付けを行っている場合、任意の貸し手が買い取ろうとすると、攻撃者は reentrancy 攻撃によって loanInfo を操作することができます。攻撃者は、買い取りを希望する貸し手が予期しないような悪い値 (例えば、非常に長い期間や0金利) を lendInfo に設定することができます。 ### How to Fix lend() メソッドにnonReentrancy を追加する。 https://www.notion.so/A-team-c88786e5e1ab4242b3d16681f5534c9b#4eb62929efce47afb5e98c4e61dd0d98 ## Title ### [H-03] 借り手が自分の貸し手となり、再貸付による買い取りから資金を奪うことができる。 ### Category Reentrancy ### Conditions 借り手は、createLoan() でローンを作成します。 同じ借り手がlend()を呼び出し、自分自身のローンに資金を供給します。借入人はlendチケットを受け取り、資金は自分自身に転送されます。 新しい貸し手が、ローンを買い取ろうとします。元の融資額+経過利息が元の貸し手(借り手と同じ人)に送られます。 checks-effects-interactionsパターンがないため、借り手は、貸し出しチケットが新しい貸し手に転送される前に、すぐにrepayAndCloseLoan()を呼び出すことが可能です。 次のコードは、新しいレンダーが、レンドチケットを受け取る前に、元のレンダーに資金を送ることを説明しています。 ```solidity } else { ERC20(loan.loanAssetContractAddress).safeTransferFrom( msg.sender, currentLoanOwner, accumulatedInterest + previousLoanAmount ); } ILendTicket(lendTicketContract).loanFacilitatorTransfer(currentLoanOwner, sendLendTicketTo, loanId); ``` 次のコードは、新しいレンダーが、レンドチケットを受け取る前に、元のレンダーに資金を送ることを説明しています。 ```solidity function repayAndCloseLoan(uint256 loanId) external override notClosed(loanId) { Loan storage loan = loanInfo[loanId]; uint256 interest = _interestOwed( loan.loanAmount, loan.lastAccumulatedTimestamp, loan.perAnumInterestRate, loan.accumulatedInterest ); address lender = IERC721(lendTicketContract).ownerOf(loanId); loan.closed = true; ERC20(loan.loanAssetContractAddress).safeTransferFrom(msg.sender, lender, interest + loan.loanAmount); IERC721(loan.collateralContractAddress).safeTransferFrom( address(this), IERC721(borrowTicketContract).ownerOf(loanId), loan.collateralTokenId ); emit Repay(loanId, msg.sender, lender, interest, loan.loanAmount); emit Close(loanId); } ``` 最後に、新しい貸し手は、この時点では何の役にも立たない貸出票を受け取る。借り手は現在、NFT、元の融資額、経過利子を所有しています。 ### Hacking details 借り手が自分のローンを貸した場合、貸し出しチケットの所有権が新しい貸し手に移る前に、返済してローンを終了させることができます。借り手は、NFT+融資額+経過利子を保有することになります。 ### How to Fix 新しい貸し手にレンドチケットを転送する行を、元の貸し手に資金を転送する行の上に移動させます。または、OpenZeppelinのreentrancyGuardを使用して、再入可能な呼び出しのリスクを完全に除去します。 必要であれば、貸し手がローンの借り手と同じアカウントであってはならないことも要求してください。 ## Title ### [M-01] 攻撃者が貸与すると、貸与者が買い取れないようなDoSを発生させることができる ### Category Denied of Service DoS ERC777 ### Conditions 1. 貸し手A(被害者)がlend()を呼び出し、貸し手B(攻撃者)のローンを買い取る。 2. 攻撃者Bが_callTokensReceivedを呼び出す。 ### Hacking details ``` solidity ERC20(loanAssetContractAddress).safeTransfer( currentLoanOwner, accumulatedInterest + previousLoanAmount ); ``` 攻撃者(貸し手)がローンを貸した場合、貸し手(被害者)が買い取ろうとすると、攻撃者は常に取引を戻すことができ、誰でも攻撃者のローンを買い取ることができないようにすることができます。 攻撃者は _callTokensReceived で loanInfoStruct(uint256 loanId) を呼び出し、loanInfo の値が変更されているかどうかをチェックして、元に戻すかどうかを決定できる。 ### How to Fix - lend()内のERC20(loanAssetContractAddress)をcurrentLoanOwner[貸し手B(攻撃者)]に転送しない。 - 移転の記録にはmapping変数を利用して送信先のアドレス情報を管理するようにする。 - transferはredeemというexternal修飾子をつけたメソッドを作成し、その中で実施。 redeem例 ``` solidity pragma solidity ^0.4.24; contract Redeemable { address owner; uint256 value; constructor() public { owner = msg.sender; } function redeem() public { require(msg.sender == owner); msg.sender.transfer(value); } } ``` ## Title [M-02] 転送トークンの手数料を扱わないプロトコル ### Category ERC20 ### Conditions 借り手は任意のトークンを指定できるので、送金時の手数料に対応したトークンでローンが組まれる可能性がある。トークンの転送に手数料が選択されている場合、`lentd()`で障害が起こります。 ### Hacking details 最初のレンダーがlend()を呼び出す場合、アセットトークンの転送手数料%がオリジネーション手数料%より大きい場合、次のコードでは2回目の転送が失敗します。 ```solidity ERC20(loanAssetContractAddress).safeTransferFrom(msg.sender, address(this), amount); uint256 facilitatorTake = amount * originationFeeRate / SCALAR; ERC20(loanAssetContractAddress).safeTransfer( IERC721(borrowTicketContract).ownerOf(loanId), amount - facilitatorTake ); ``` ### How to Fix 融資額に基づいて`originationFee`を計算し、貸し手が`feeOnTransfer <= originationFee`となるように送金する ## Title ### [M-03] closeLoan()でsendCollateralToのチェックが外れており、ユーザーの担保NFTが凍結される可能性がある。 ### Category ERC721 ### Conditions 1. sendCollateralToコントラクトがERC721をサポートしない時。 2. transferFrom()しておりERC721規格をサポートしているか確認できない。 ### Hacking details ``` solidity function closeLoan(uint256 loanId, address sendCollateralTo) external override notClosed(loanId) { require(IERC721(borrowTicketContract).ownerOf(loanId) == msg.sender, "NFTLoanFacilitator: borrow ticket holder only"); Loan storage loan = loanInfo[loanId]; require(loan.lastAccumulatedTimestamp == 0, "NFTLoanFacilitator: has lender, use repayAndCloseLoan"); loan.closed = true; IERC721(loan.collateralContractAddress).transferFrom(address(this), sendCollateralTo, loan.collateralTokenId); emit Close(loanId); } ``` sendCollateralTo は、closeLoan()が呼ばれたときに担保 NFT を受け取ります。ただし、sendCollateralTo が ERC721 をサポートしないコントラクトのアドレスである場合、担保 NFT はコントラクト内でFrozenする。 ```solidity IERC721(loan.collateralContractAddress).safeTransferFrom(address(this), sendCollateralTo, loan.collateralTokenId); ``` ### How to Fix transferFrom を safeTransferFrom に修正する。 ## Title [M-05] 借り手がcloseLoanではなくrepayAndCloseLoanを呼び出すと資金が失われる ### Category no requier ### Conditions 融資の貸し手がいない場合ユーザーは資金を失う可能性がある ### Hacking details この場合`closeLoan`を使用する必要があり、`replayAndcloseLoan`の呼び出しを禁止する必要がある ### How to Fix 返済するものがあるかチェックを加える ```solidity require(loan.lastAccumulatedTimestamp > 0, "loan was never matched by a lender. use closeLoan instead"); uint256 interest = _interestOwed( loan.loanAmount, loan.lastAccumulatedTimestamp, loan.perAnumInterestRate, loan.accumulatedInterest ); ``` ## Title [M-07] mintBorrowTicketToは、onERC721Receivedメソッドを持たない契約である可能性があるため、BorrowTicket NFTが凍結され、ユーザーの資金が危険にさらされる可能性があります。 ### Category safeMint ### Conditions mintBorrowTicketTo が onERC721Received メソッドを実装していない契約であっても、現在の createLoan() の実装では、tx は正常に実行され、ローンが作成されます。 ### Hacking details これは、mintBorrowTicketTo が ERC721 を適切に処理できない場合に問題となる可能性があります。 ### How to Fix `NFTLoanTicket.sol#mint()`で`safeMint`を使用する ```solidity function mint(address to, uint256 tokenId) external override loanFacilitatorOnly { _safeMint(to, tokenId); } ```