# Day 4 D-team ## [H-2] 貸し手がBuyOutを試みた際に、currentLoanOwnerはloanInfoを操作することができる ### ■ カテゴリー Re-entrancy Attack ### ■ 条件 攻撃者（借り手）が融資を得るためにlend()を呼び出します。 この状態で、貸し手がBuyOutを試みた時に、攻撃者はRe-entrancy Attackにより、loanInfoを操作することができる。 ### ■ ハッキングの詳細 被害者（貸し手）がlend()を呼び出します。 lend()関数内では、 accumulatedInterest + previousLoanAmount を ERC20(loanAssetContractAddress).safeTransfer()関数を使って、currentLoanOwner (攻撃者) に送金します。 ERC777規格であるloanAssetContractAddressの転送は_callTokensReceived()関数を呼ぶ必要があるので、攻撃者が以下のようなパラメータでRe-Entrancy Attackを実行すると、lend() を何度も呼び出せてしまいます。 loanId: 同じローンID interestRate（利率）: 悪い値（例：0）に設定される。 amount: 同じ金額 durationSeconds（貸出期間）: 不正な値に設定される (例: 長い durationSeconds) sendLendTicketTo: 攻撃者 (currentLoanOwner) のアドレスと同じ。 これで、loanInfoの変数が悪い値に変更され、被害者は貸し出しチケットを受け取りますが、ローン期間が操作され、ローンを戻せなくなります。 ### ■ 修正方法 lend()関数にnonReentrantのモディファイアを付けると、Re-Entrancy Attackを防止できます。 ## [H-3] 借り手は自分の貸し手になり、再入可能性により資金を盗むことが可能 ### ■ カテゴリー Reentrancy ### ■ 条件 * checks-effects-interactions パターンに沿っていない ### ■ ハッキングの詳細 借り手が***自分***へローンを貸す場合、貸出チケットの所有権が新しい貸主に譲渡される前にローンを返済してクローズすることができる。 この時借り手は、NFT + ローン額 + 経過利息を得ることができる。 check → interaction(送金) → effects（チケットの所有権変更）になっている ### ■ 修正方法 レンドチケットを新しい貸し手に転送するための行を、元の貸し手に資金を転送するための行の上に移動する。 または、OpenZeppelin の reentrancyGuard を使用して、再入呼び出しのリスクを完全に取り除く。 貸し手がローンの借り手と同じアカウントであってはならないことを要求する。 ## [M-1]攻撃者がローンを貸し出した場合、どの貸し手もそのローンを買い取ることができないDoSを発動することができる ### ■ カテゴリー ERC20 ### ■ 条件 攻撃者（貸し手）がローンを貸した場合、他の貸し手がそのローンを買い取ろうとすると、攻撃者は常に取引を元に戻すことができ、誰も攻撃者のローンを買い取れないようにすることができます。 ### ■ ハッキングの詳細 被害者は、攻撃者のローンを買い取ろうと、lend()を呼び出します。 lend()関数内では、常に accumulatedInterest + previousLoanAmount をERC20(loanAssetContractAddress).safeTransfer()関数を使って、currentLoanOwner（攻撃者）に送金します。 loanAssetContractAddressの転送がERC777の場合、攻撃者が操作可能な_callTokensReceivedを呼び出し、必ず元に戻すことになる。 NFTLoanFacilitator は safeTransfer と safeTransferFrom を使って戻り値をチェックするため、被害者の取引も元に戻されることになります。これにより、誰も攻撃者のローンを買い取ることができなくなります。 callTokensReceivedでは、攻撃者は買い取りトランザクションを元に戻すだけで、repayAndCloseLoanは成功したままにしておきたいです。攻撃者は _callTokensReceived で loanInfoStruct(uint256 loanId) を呼び出し、loanInfo の値が変更されているかどうかをチェックして、それを戻すかどうかを決定することができます。 ### ■ 修正方法 lend()関数内でERC20(loanAssetContractAddress)をcurrentLoanOwnerに転送しない。グローバルマッピングを使用して貸し手の償還を記録し、externalな関数redeemを追加してERC20(loanAssetContractAddress)を転送する。 ## [M-2] 送金手数料をサポートするトークンを考慮する ### ■ カテゴリー Token type ### ■ 条件 * 送金手数料をサポートするトークンを利用する ### ■ ハッキングの詳細 - `originationFee = 2%` Max fee is 5% per comments - `feeOnTransfer = 3%` - `amount = 100 tokens` - Lender transfers `amount` - `NFTLoanFacilitator` receives `97`. （amount - feeOnTransfer, 100 - 3） - `facilitatorTake = 2` - `NFTLoanFacilitator` attempts to send `100 - 2` to borrower, but only has `97`. - Execution reverts. ### ■ 修正方法 リクエストされたローンの金額に基づいて originationFee を計算し、feeOnTransfer <= originationFee となるように貸し手がより大きな値を送信できるようにする ## [M-3]closeLoan()でsendCollateralToがチェックされていないため、ユーザーの担保NFTが凍結される可能性があります。Collateral＝担保 ### ■ カテゴリー ERC721 ### ■ 条件 sendCollateralTo は、closeLoan()が呼ばれたときに担保 NFT を受け取ります。ただし、送信先が ERC721 をサポートしないコントラクトのアドレスである場合、担保NFTはコントラクト内で凍結されてしまいます。 EIP-721のドキュメントによると、 ウォレット/ブローカー/オークションアプリケーションが、ERC721に準拠したNFTの安全な転送を受け入れる場合、ERC721をサポートしたウォレットインターフェースを実装しなければなりません。 ### ■ ハッキングの詳細 transferFromを使用してしまっている `IERC721(loan.collateralContractAddress).transferFrom(address(this), sendCollateralTo, loan.collateralTokenId);` ### ■ 修正方法 safeTransferFromを使用する `IERC721(loan.collateralContractAddress).safeTransferFrom(address(this), sendCollateralTo, loan.collateralTokenId);` ## [M-5] `closeLoan` の代わりに `repayAndCloseLoan` を呼び出すと、借り手は資金を失う ### ■ カテゴリー No check ### ■ 条件 * requireでのチェックが漏れている ### ■ ハッキングの詳細 貸し手とユーザー（借り手？）がいる。 貸し手がいなくなった状態で`repayAndCloseLoan`（支払いを行いloanを閉じる）を呼んだ場合にrevertが実行されない。また、送金した資金を失うことになる。 ### ■ 修正方法 返却する値（loanの残り？）があることを確認する。 下記のような`require`を追記している。 ```solidity require(loan.lastAccumulatedTimestamp > 0, "loan was never matched by a lender. use closeLoan instead"); ``` ## [M-7]mintBorrowTicketTo(送り先)は、onERC721Received メソッドを持たないコントラクトである可能性があり、BorrowTicket NFT が凍結され、ユーザーの資金を危険にさらす可能性があります。 ### ■ カテゴリー ERC721 ### ■ 条件 mintBorrowTicketTo が onERC721Received メソッドを実装していないコントラクトであっても、現在の createLoan() の実装では、トランザクションは正常に実行され、ローンが作成されてしまいます。 これは、mintBorrowTicketTo が ERC721 を適切に処理できないコントラクトである場合に問題となることがあります。 ### ■ ハッキングの詳細 IERC721Mintable(borrowTicketContract).mint(mintBorrowTicketTo, id); `function mint(address to, uint256 tokenId) external override loanFacilitatorOnly { _mint(to, tokenId); }` ### ■ 修正方法 NFTLoanTicket.solのmint()でsafeMint()の使用を検討してください。 `function mint(address to, uint256 tokenId) external override loanFacilitatorOnly { _safeMint(to, tokenId); }`