Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

LOW

sma4

Low Risk Issues

Number Issue Instances
[LOW-01] Use Ownable2Step rather than Ownable 4
[LOW-02] Functions which set address state variables should have zero address checks 2
[LOW-03] Unbounded state variable arrays exist within the project which are iterated upon 1
[LOW-04] Potential division by zero should have zero checks in place 3
[LOW-05] Contracts do not use their OZ upgradable counterparts 3
[LOW-06] Use of onlyOwner functions can be lost 4
[LOW-07] Constant decimal values 3
[LOW-08] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards 12
[LOW-09] Calculation will revert when totalSupply() returns zero 1
[LOW-10] Missing zero address check in constructor 3

[LOW‑01] Use Ownable2Step rather than Ownable

Ownable2Step and Ownable2StepUpgradeable prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.

File: src/Beedle.sol
​
9 contract Beedle is Ownable, ERC20, ERC20Permit, ERC20Votes {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L9

File: src/Lender.sol
​
10 contract Lender is Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L10

File: src/Staking.sol
​
11 contract Staking is Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L11

File: src/utils/Ownable.sol
​
4 abstract contract Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/utils/Ownable.sol#L4

[LOW-02] Functions which set address state variables should have zero address checks

File: src/Lender.sol
​
100 function setFeeReceiver(address _feeReceiver) external onlyOwner {
feeReceiver = _feeReceiver;
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L100-L102

File: src/utils/Ownable.sol
​
19 function transferOwnership(address _owner) public virtual onlyOwner {
owner = _owner;
emit OwnershipTransferred(msg.sender, _owner);
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/utils/Ownable.sol#L14-L17

[LOW-03] Unbounded state variable arrays exist within the project which are iterated upon

Unbounded arrays in Solidity can cause gas-related issues due to their potential for excessive growth, leading to increased computational complexity and resource consumption. Since Ethereum's gas fees are determined by the amount of computational effort required to execute a transaction, operations involving large, unbounded arrays can result in prohibitively high costs for users. Additionally, if a function iterates over an unbounded array, it may exceed the gas limit set by the network, causing the transaction to fail. To avoid these issues, developers should use bounded arrays or alternative data structures like mappings, ensuring efficient resource management and a better user experience.

file: src/Lender.sol
​
276 loans.push(loan);
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L#L276

[LOW-04] Potential division by zero should have zero checks in place

Implement a zero address check for found instances

File: src/Staking.sol
68 uint256 _ratio = _diff * 1e18 / totalSupply;
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L68

File: src/Lender.sol
​
246 uint256 loanRatio = (debt * 10 ** 18) / collateral;
​
384 uint256 loanRatio = (totalDebt * 10 ** 18) / loan.collateral;

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L246

[LOW-05] Contracts do not use their OZ upgradable counterparts

Using the upgradeable counterpart of the OpenZeppelin (OZ) library in Solidity is beneficial for creating contracts that can be updated in the future. OpenZeppelin's upgradeable contracts library is designed with proxy patterns in mind, which allow the logic of contracts to be upgraded while preserving the contract's state and address. This can be crucial for long-lived contracts where future requirements or improvements may not be fully known at the time of deployment. The upgradeable OZ contracts also include protection against a class of vulnerabilities related to initialization of storage variables in upgradeable contracts. Hence, it's a good idea to use them when developing contracts that may need to be upgraded in the future, as they provide a solid foundation for secure and upgradeable smart contracts.

File: src/Beedle.sol
​
5 import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
6 import {ERC20Permit} from "openzeppelin-contracts/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
7 import {ERC20Votes} from "openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Votes.sol";
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L5

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L6

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L7

[LOW-06] Use of onlyOwner functions can be lost

In Solidity, renouncing ownership of a contract essentially transfers ownership to the zero address. This is an irreversible operation and has considerable security implications. If the renounceOwnership function is used, the contract will lose the ability to perform any operations that are limited to the owner. This can be problematic if there are any bugs, flaws, or unexpected events that require owner intervention to resolve. Therefore, in some instances, it is better to disable or omit the renounceOwnership function, and instead implement a secure transferOwnership function. This way, if necessary, ownership can be transferred to a new, trusted party without losing the potential for administrative intervention.

File: src/Beedle.sol
​
9 contract Beedle is Ownable, ERC20, ERC20Permit, ERC20Votes {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L9

File: src/Lender.sol
​
10 contract Lender is Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L10

File: src/Staking.sol
​
11 contract Staking is Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L11

File: src/utils/Ownable.sol
​
4 abstract contract Ownable {
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/utils/Ownable.sol#L4

[LOW-07] Constant decimal values

The use of fixed decimal values such as 1e18 or 1e8 in Solidity contracts can lead to inaccuracies, bugs, and vulnerabilities, particularly when interacting with tokens having different decimal configurations. Not all ERC20 tokens follow the standard 18 decimal places, and assumptions about decimal places can lead to miscalculations.

Resolution: Always retrieve and use the decimals() function from the token contract itself when performing calculations involving token amounts. This ensures that your contract correctly handles tokens with any number of decimal places, mitigating the risk of numerical errors or under/overflows that could jeopardize contract integrity and user funds

File: src/Staking.sol
​
88 uint256 _share = _supplied * _delta / 1e18;
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L88

File: src/Beedle.sol
​
12 _mint(msg.sender, 1_000_000_000 * 1e18);
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L12

File: src/Staking.sol
​
​
68 uint256 _ratio = _diff * 1e18 / totalSupply;
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L68

[LOW-08] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards

While adherence to the check-effects-interaction pattern is commendable, the absence of a reentrancy guard in functions, especially where transfer hooks might be present, can expose the protocol users to risks of read-only reentrancies. Such reentrancy vulnerabilities can be exploited to execute malicious actions even without altering the contract state. Without a reentrancy guard, the only potential mitigation would be to blocklist the entire protocol - an extreme and disruptive measure. Therefore, incorporating a reentrancy guard into these functions is vital to bolster security, as it helps protect against both traditional reentrancy attacks and read-only reentrancies, ensuring robust and safe protocol operations.

File: src/Beedle.sol
​
15 function _afterTokenTransfer(address from, address to, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._afterTokenTransfer(from, to, amount); // <= FOUND
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Beedle.sol#L15-L20

File: src/Staking.sol
​
38 function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount); // <= FOUND
updateFor(msg.sender);
balances[msg.sender] += _amount;
}
​
46 function withdraw(uint _amount) external {
updateFor(msg.sender);
balances[msg.sender] -= _amount;
TKN.transfer(msg.sender, _amount); // <= FOUND
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L38-L42

File: src/Fees.sol
​
26 function sellProfits(address _profits) public {
require(_profits != WETH, "not allowed");
uint256 amount = IERC20(_profits).balanceOf(address(this));
​
ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
.ExactInputSingleParams({
tokenIn: _profits,
tokenOut: WETH,
fee: 3000,
recipient: address(this),
deadline: block.timestamp,
amountIn: amount,
amountOutMinimum: 0,
sqrtPriceLimitX96: 0
});
​
amount = swapRouter.exactInputSingle(params);
IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this))); // <= FOUND
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Fees.sol#L26-L44

File: src/Lender.sol
​
130 function setPool(Pool calldata p) public returns (bytes32 poolId) {
// validate the pool
if (
p.lender != msg.sender ||
p.minLoanSize == 0 ||
p.maxLoanRatio == 0 ||
p.auctionLength == 0 ||
p.auctionLength > MAX_AUCTION_LENGTH ||
p.interestRate > MAX_INTEREST_RATE
) revert PoolConfig();
​
// check if they already have a pool balance
poolId = getPoolId(p.lender, p.loanToken, p.collateralToken);
​
// you can't change the outstanding loans
if (p.outstandingLoans != pools[poolId].outstandingLoans)
revert PoolConfig();
​
uint256 currentBalance = pools[poolId].poolBalance;
​
if (p.poolBalance > currentBalance) {
// if new balance > current balance then transfer the difference from the lender
IERC20(p.loanToken).transferFrom( // <= FOUND
p.lender,
address(this),
p.poolBalance - currentBalance
);
} else if (p.poolBalance < currentBalance) {
// if new balance < current balance then transfer the difference back to the lender
IERC20(p.loanToken).transfer( // <= FOUND
p.lender,
currentBalance - p.poolBalance
);
}
​
emit PoolBalanceUpdated(poolId, p.poolBalance);
​
if (pools[poolId].lender == address(0)) {
// if the pool doesn't exist then create it
emit PoolCreated(poolId, p);
} else {
// if the pool does exist then update it
emit PoolUpdated(poolId, p);
}
​
pools[poolId] = p;
}
​
​
198 function removeFromPool(bytes32 poolId, uint256 amount) external {
if (pools[poolId].lender != msg.sender) revert Unauthorized();
if (amount == 0) revert PoolConfig();
_updatePoolBalance(poolId, pools[poolId].poolBalance - amount);
// transfer the loan tokens from the contract to the lender
IERC20(pools[poolId].loanToken).transfer(msg.sender, amount); // <= FOUND
}
​
232 function borrow(Borrow[] calldata borrows) public {
for (uint256 i = 0; i < borrows.length; i++) {
bytes32 poolId = borrows[i].poolId;
uint256 debt = borrows[i].debt;
uint256 collateral = borrows[i].collateral;
// get the pool info
Pool memory pool = pools[poolId];
// make sure the pool exists
if (pool.lender == address(0)) revert PoolConfig();
// validate the loan
if (debt < pool.minLoanSize) revert LoanTooSmall();
if (debt > pool.poolBalance) revert LoanTooLarge();
if (collateral == 0) revert ZeroCollateral();
// make sure the user isn't borrowing too much
uint256 loanRatio = (debt * 10 ** 18) / collateral;
if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();
// create the loan
Loan memory loan = Loan({
lender: pool.lender,
borrower: msg.sender,
loanToken: pool.loanToken,
collateralToken: pool.collateralToken,
debt: debt,
collateral: collateral,
interestRate: pool.interestRate,
startTimestamp: block.timestamp,
auctionStartTimestamp: type(uint256).max,
auctionLength: pool.auctionLength
});
// update the pool balance
_updatePoolBalance(poolId, pools[poolId].poolBalance - debt);
pools[poolId].outstandingLoans += debt;
// calculate the fees
uint256 fees = (debt * borrowerFee) / 10000;
// transfer fees
IERC20(loan.loanToken).transfer(feeReceiver, fees); // <= FOUND
// transfer the loan tokens from the pool to the borrower
IERC20(loan.loanToken).transfer(msg.sender, debt - fees); // <= FOUND
// transfer the collateral tokens from the borrower to the contract
IERC20(loan.collateralToken).transferFrom( // <= FOUND
msg.sender,
address(this),
collateral
);
loans.push(loan);
emit Borrowed(
msg.sender,
pool.lender,
loans.length - 1,
debt,
collateral,
pool.interestRate,
block.timestamp
);
}
}
​
292 function repay(uint256[] calldata loanIds) public {
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
// get the loan info
Loan memory loan = loans[loanId];
// calculate the interest
(
uint256 lenderInterest,
uint256 protocolInterest
) = _calculateInterest(loan);
​
bytes32 poolId = getPoolId(
loan.lender,
loan.loanToken,
loan.collateralToken
);
​
// update the pool balance
_updatePoolBalance(
poolId,
pools[poolId].poolBalance + loan.debt + lenderInterest
);
pools[poolId].outstandingLoans -= loan.debt;
​
// transfer the loan tokens from the borrower to the pool
IERC20(loan.loanToken).transferFrom( // <= FOUND
msg.sender,
address(this),
loan.debt + lenderInterest
);
// transfer the protocol fee to the fee receiver
IERC20(loan.loanToken).transferFrom( // <= FOUND
msg.sender,
feeReceiver,
protocolInterest
);
// transfer the collateral tokens from the contract to the borrower
IERC20(loan.collateralToken).transfer( // <= FOUND
loan.borrower,
loan.collateral
);
emit Repaid(
msg.sender,
loan.lender,
loanId,
loan.debt,
loan.collateral,
loan.interestRate,
loan.startTimestamp
);
// delete the loan
delete loans[loanId];
}
}
​
591 function refinance(Refinance[] calldata refinances) public {
for (uint256 i = 0; i < refinances.length; i++) {
uint256 loanId = refinances[i].loanId;
bytes32 poolId = refinances[i].poolId;
bytes32 oldPoolId = keccak256(
abi.encode(
loans[loanId].lender,
loans[loanId].loanToken,
loans[loanId].collateralToken
)
);
uint256 debt = refinances[i].debt;
uint256 collateral = refinances[i].collateral;
​
// get the loan info
Loan memory loan = loans[loanId];
// validate the loan
if (msg.sender != loan.borrower) revert Unauthorized();
​
// get the pool info
Pool memory pool = pools[poolId];
// validate the new loan
if (pool.loanToken != loan.loanToken) revert TokenMismatch();
if (pool.collateralToken != loan.collateralToken)
revert TokenMismatch();
if (pool.poolBalance < debt) revert LoanTooLarge();
if (debt < pool.minLoanSize) revert LoanTooSmall();
uint256 loanRatio = (debt * 10 ** 18) / collateral;
if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();
​
// calculate the interest
(
uint256 lenderInterest,
uint256 protocolInterest
) = _calculateInterest(loan);
uint256 debtToPay = loan.debt + lenderInterest + protocolInterest;
​
// update the old lenders pool
_updatePoolBalance(
oldPoolId,
pools[oldPoolId].poolBalance + loan.debt + lenderInterest
);
pools[oldPoolId].outstandingLoans -= loan.debt;
​
// now lets deduct our tokens from the new pool
_updatePoolBalance(poolId, pools[poolId].poolBalance - debt);
pools[poolId].outstandingLoans += debt;
​
if (debtToPay > debt) {
// we owe more in debt so we need the borrower to give us more loan tokens
// transfer the loan tokens from the borrower to the contract
IERC20(loan.loanToken).transferFrom( // <= FOUND
msg.sender,
address(this),
debtToPay - debt
);
} else if (debtToPay < debt) {
// we have excess loan tokens so we give some back to the borrower
// first we take our borrower fee
uint256 fee = (borrowerFee * (debt - debtToPay)) / 10000;
IERC20(loan.loanToken).transfer(feeReceiver, fee); // <= FOUND
// transfer the loan tokens from the contract to the borrower
IERC20(loan.loanToken).transfer(msg.sender, debt - debtToPay - fee); // <= FOUND
}
// transfer the protocol fee to governance
IERC20(loan.loanToken).transfer(feeReceiver, protocolInterest); // <= FOUND
​
// update loan debt
loans[loanId].debt = debt;
// update loan collateral
if (collateral > loan.collateral) {
// transfer the collateral tokens from the borrower to the contract
IERC20(loan.collateralToken).transferFrom(
msg.sender,
address(this),
collateral - loan.collateral
);
} else if (collateral < loan.collateral) {
// transfer the collateral tokens from the contract to the borrower
IERC20(loan.collateralToken).transfer( // <= FOUND
msg.sender,
loan.collateral - collateral
);
}
​
emit Repaid(
msg.sender,
loan.lender,
loanId,
debt,
collateral,
loan.interestRate,
loan.startTimestamp
);
​
loans[loanId].collateral = collateral;
// update loan interest rate
loans[loanId].interestRate = pool.interestRate;
// update loan start timestamp
loans[loanId].startTimestamp = block.timestamp;
// update loan auction start timestamp
loans[loanId].auctionStartTimestamp = type(uint256).max;
// update loan auction length
loans[loanId].auctionLength = pool.auctionLength;
// update loan lender
loans[loanId].lender = pool.lender;
// update pool balance
pools[poolId].poolBalance -= debt;
emit Borrowed(
msg.sender,
pool.lender,
loanId,
debt,
collateral,
pool.interestRate,
block.timestamp
);
emit Refinanced(loanId);
}
}
​
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L130-L176

[LOW-09] Calculation will revert when totalSupply() returns zero

In the instance where the function totalSupply() returns zero, it will inevitably lead to a division by zero error when used in mathematical operations, causing the transaction to fail and potentially disrupting contract functionality. This situation can inadvertently serve as a blocking mechanism, preventing valid transactions and operations. It's crucial to accommodate this special scenario in your code. One resolution could be implementing condition checks in your function to detect a zero totalSupply() and handle it differently, perhaps by returning a specific value or altering the operational flow, thus ensuring that transactions do not revert and the contract functions smoothly even in this edge case.

File: src/Staking.sol
​
68 uint256 _ratio = _diff * 1e18 / totalSupply;
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L68

[LOW-10] Missing zero address check in constructor

In Solidity, constructors often take address parameters to initialize important components of a contract, such as owner or linked contracts. However, without a check, there's a risk that an address parameter could be mistakenly set to the zero address (0x0). This could occur due to a mistake or oversight during contract deployment. A zero address in a crucial role can cause serious issues, as it cannot perform actions like a normal address, and any funds sent to it are irretrievable. Therefore, it's crucial to include a zero address check in constructors to prevent such potential problems. If a zero address is detected, the constructor should revert the transaction.

File: src/Fees.sol
​
19 constructor(address _weth, address _staking) {
WETH = _weth;
staking = _staking;
}

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Fees.sol#L19-L22

File: src/Staking.sol
​
31 constructor(address _token, address _weth) Ownable(msg.sender) {
TKN = IERC20(_token);
WETH = IERC20(_weth);
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Staking.sol#L31-L34

File: src/utils/Ownable.sol
14 constructor(address _owner) {
owner = _owner;
emit OwnershipTransferred(address(0), _owner);
}
​

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/utils/Ownable.sol#L14-L17

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.