Platform: Code4rena
Start Date: 25/10/2022
Pot Size: $50,000 USDC
Total HM: 18
Participants: 127
Period: 5 days
Judge: 0xean
Total Solo HM: 9
Id: 175
League: ETH
Rank: 62/127
Findings: 2
Award: $55.74
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0x1f8b
Also found by: 0xNazgul, 0xSmartContract, Aymen0909, B2, Bnke0x0, Deivitto, Diana, Dinesh11G, ElKu, JC, Josiah, Rahoz, RaymondFam, ReyAdmirado, Rolezn, Waze, __141345__, adriro, aphak5010, brgltd, c3phas, c7e7eff, carlitox477, cducrest, ch0bu, chrisdior4, cryptonue, cryptostellar5, cylzxje, d3e4, delfin454000, enckrish, evmwanderer, fatherOfBlocks, gogo, hansfriese, horsefacts, immeas, leosathya, lukris02, neumo, oyc_109, pedr02b2, rbserver, robee, rotcivegaf, rvierdiiev, sakshamguruji, shark, simon135, tnevler, trustindistrust, wagmi
36.7345 USDC - $36.73
address(0x0) when assigning values to address state variablesMissing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly.
operator = _operator;
File: src/BorrowController.sol (line 14)
operator = _operator;
File: src/DBR.sol (line 39)
gov = _gov; chair = _chair; supplyCeiling = _supplyCeiling;
File: src/Fed.sol (line 39-41)
gov = _gov; lender = _lender; pauseGuardian = _pauseGuardian; escrowImplementation = _escrowImplementation;
File: src/Market.sol (line 77-80)
operator = _operator;
File: src/Oracle.sol (line 32)
require() statement should have descriptive reason stringsrequire(globalSupply <= supplyCeiling);
File: src/Fed.sol (line 93)
require(msg.sender == beneficiary);
File: src/escrows/GovTokenEscrow.sol (line 67)
require(msg.sender == beneficiary);
File: src/escrows/INVEscrow.sol (line 91)
msg.sender instead of tx.originif(msgSender == tx.origin) return true;
Cross-Chain Replay attackStoring the block.chainid is not safe.
INITIAL_CHAIN_ID = block.chainid;
INITIAL_CHAIN_ID = block.chainid;
return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : computeDomainSeparator();
block.chainid,
return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : computeDomainSeparator();
block.chainid,
ecrecover() allows signature malleabilityUse the recover function from OpenZeppelin’s ECDSA library for signature verification.
address recoveredAddress = ecrecover(
address recoveredAddress = ecrecover(
address recoveredAddress = ecrecover(
initialize functions can be Front-runThis function can be called by everyone. So an attacker may watch the mempool for this function and may frontrun it by supplying more gas. This may result in unintended behaviour.
function initialize(IERC20 _token, address _beneficiary) public {
function initialize(IERC20 _token, address _beneficiary) public {
function initialize(IERC20 _token, address _beneficiary) public {
Solidity could truncate the results, performing multiplication before division will prevent rounding/truncation in solidity math.
uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
return collateralValue * collateralFactorBps / 10000;
uint newBorrowingPower = normalizedPrice * collateralFactorBps / 10000;
Other instances of this issue are
TODO commentsCode architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.
xINV = _xINV; // TODO: Test whether an immutable variable will persist across proxies
///@audit: addres @param deniedContract The addres of the denied contract
///@audit oprator @notice Sets pending operator of the contract. Operator role must be claimed by the new oprator. Only callable by Operator.
///@audit allowe @notice Removes a minter from the set of addresses allowe to mint DBR tokens. Only callable by Operator.
///@audit th @notice Get the DBR deficit of an address. Will return 0 if th user has zero DBR or more.
///@audit mus @dev Collateral factor mus be set below 100%
///@audit liqudation @param _liquidationIncentiveBps The new liqudation incentive set in basis points. 1 = 0.01%
block.timestampBlock timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.
uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
Other instances of this issue are :
indexed fieldsEach event should use three indexed fields if there are three or more fields.
event Transfer(address indexed from, address indexed to, uint256 amount);
event Transfer(address indexed from, address indexed to, uint256 amount);
event Withdraw(address indexed account, address indexed to, uint amount); event Repay(address indexed account, address indexed repayer, uint amount); event ForceReplenish(address indexed account, address indexed replenisher, uint deficit, uint replenishmentCost, uint replenisherReward); event Liquidate(address indexed account, address indexed liquidator, uint repaidDebt, uint liquidatorReward);
NatSpec is incomplete/// @audit Missing: '@return' /** @notice Internal function for creating an escrow for users to deposit collateral in. @dev Uses create2 and minimal proxies to create the escrow at a deterministic address @param user The address of the user to create an escrow for. */ function createEscrow(address user) internal returns (IEscrow instance) {
/// @audit Missing: '@return' /** @notice Internal function for getting the escrow of a user. @dev If the escrow doesn't exist, an escrow contract is deployed. @param user The address of the user owning the escrow. */ function getEscrow(address user) internal returns (IEscrow) {
/// @audit Missing: '@return' /** @notice View function for predicting the deterministic escrow address of a user. @dev Only use deposit() function for deposits and NOT the predicted escrow address unless you know what you're doing @param user Address of the user owning the escrow. */ function predictEscrow(address user) public view returns (IEscrow predicted) {
/// @audit Missing: '@return' /** @notice View function for getting the dollar value of the user's collateral in escrow for the market. @param user Address of the user. */ function getCollateralValue(address user) public view returns (uint) {
/// @audit Missing: '@return' /** @notice Internal function for getting the dollar value of the user's collateral in escrow for the market. @dev Updates the lowest price comparisons of the pessimistic oracle @param user Address of the user. */ function getCollateralValueInternal(address user) internal returns (uint) {
/// @audit Missing: '@return' /** @notice View function for getting the credit limit of a user. @dev To calculate the available credit, subtract user debt from credit limit. @param user Address of the user. */ function getCreditLimit(address user) public view returns (uint) {
/// @audit Missing: '@return' /** @notice Internal function for getting the credit limit of a user. @dev To calculate the available credit, subtract user debt from credit limit. Updates the pessimistic oracle. @param user Address of the user. */ function getCreditLimitInternal(address user) internal returns (uint) {
/// @audit Missing: '@return' /** @notice Internal function for getting the withdrawal limit of a user. The withdrawal limit is how much collateral a user can withdraw before their loan would be underwater. Updates the pessimistic oracle. @param user Address of the user. */ function getWithdrawalLimitInternal(address user) internal returns (uint) {
/// @audit Missing: '@return' /** @notice View function for getting the withdrawal limit of a user. The withdrawal limit is how much collateral a user can withdraw before their loan would be underwater. @param user Address of the user. */ function getWithdrawalLimit(address user) public view returns (uint) {
/// @audit Missing: '@return' /** @notice View function for getting the amount of liquidateable debt a user holds. @param user The address of the user. */ function getLiquidatableDebt(address user) public view returns (uint) {
#0 - c4-judge
2022-11-07T19:53:51Z
0xean marked the issue as grade-b
🌟 Selected for report: pfapostol
Also found by: 0x1f8b, 0xRoxas, 0xSmartContract, Amithuddar, Aymen0909, B2, Bnke0x0, Chandr, CloudX, Deivitto, Diana, Dinesh11G, ElKu, HardlyCodeMan, JC, JrNet, KoKo, Mathieu, Ozy42, Rahoz, RaymondFam, ReyAdmirado, Rolezn, Shinchan, __141345__, adriro, ajtra, aphak5010, ballx, c3phas, carlitox477, ch0bu, chaduke, cryptostellar5, djxploit, durianSausage, enckrish, exolorkistis, fatherOfBlocks, gogo, horsefacts, kaden, karanctf, leosathya, martin, mcwildy, oyc_109, ret2basic, robee, sakman, sakshamguruji, shark, skyle, tnevler
19.0072 USDC - $19.01
public functions not called the contract should be declared external insteadcontracts are allowed to override their parents’ functions and change the visibility from externalto public.
function setOperator(address _operator) public onlyOperator { operator = _operator; }
File: src/BorrowController.sol (line 26)
function setPendingOperator(address newOperator_) public onlyOperator {
File: src/DBR.sol (line 53)
function changeGov(address _gov) public {
File: /src/Fed.sol (line 48)
function claimOperator() public {
File: /src/Oracle.sol (line 66)
function balance() public view returns (uint) {
File: src/escrows/GovTokenEscrow.sol (line 52)
function pay(address recipient, uint amount) public {
File: src/escrows/INVEscrow.sol (line 59)
function pay(address recipient, uint amount) public {
File: src/escrows/SimpleERC20Escrow.sol (line 36)
Some Instances of this issue are:
storage variables in memory to save GasAnytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.
///@audit: totalDueTokensAccrued is used more than one time if(totalDueTokensAccrued > _totalSupply) return 0; return _totalSupply - totalDueTokensAccrued;
File: /src/DBR.sol (line 110-111)
///@audit: pendingOperator is used more than one time require(msg.sender == pendingOperator, "ONLY PENDING OPERATOR"); operator = pendingOperator;
File: /src/Oracle.sol (line 67-68)
///@audit: collateralFactorBps is used more than one time if(collateralFactorBps == 0) return 0; uint minimumCollateral = debt * 1 ether / oracle.getPrice(address(collateral), collateralFactorBps) * 10000 / collateralFactorBps;
File: /src/Market.sol (line 359-360)
///@audit: collateralFactorBps is used more than one time if(collateralFactorBps == 0) return 0; uint minimumCollateral = debt * 1 ether / oracle.viewPrice(address(collateral), collateralFactorBps) * 10000 / collateralFactorBps;
File: /src/Market.sol (line 376-377)
///@audit: liquidationFeeBps is used more than one time if(liquidationFeeBps > 0) { uint liquidationFee = repaidDebt * 1 ether / price * liquidationFeeBps / 10000;
File: /src/Market.sol (line 605-606)
calldata instead of memoryIf a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.
Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.
string memory _name, string memory _symbol,
File: /src/DBR.sol (line 32-33)
address mappings can be combined into a single mapping of an address to a struct, where appropriate.Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot
mapping (address => FeedData) public feeds; mapping (address => uint) public fixedPrices; mapping (address => mapping(uint => uint)) public dailyLows; /
File: /src/Oracle.sol (line 25-27)
mapping(address => uint256) public nonces; mapping (address => bool) public minters; mapping (address => bool) public markets; mapping (address => uint) public debts; // user => debt across all tracked markets mapping (address => uint) public dueTokensAccrued; // user => amount of due tokens accrued mapping (address => uint) public lastUpdated; // user => last update timestamp
File: /src/DBR.sol (line 23-28)
mapping(address => uint256) public balances; mapping(address => mapping(address => uint256)) public allowance;
File: /src/DBR.sol (line 19-20)
mapping (address => IEscrow) public escrows; // user => escrow mapping (address => uint) public debts; // user => debt mapping(address => uint256) public nonces; // user => nonce
File: /src/Market.sol (line 57-59)
emit from storage is more expensiveEmmitting an event using storage data is more expensive
emit ChangeOperator(operator);
File: /src/DBR.sol (line 74)
emit ChangeOperator(operator);
File: /src/Oracle.sol (line 70)
require() statements that use && saves gas (even a single &&)Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.
require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps < 10000, "Invalid liquidation incentive");
File: /src/Market.sol (line 75)
require(_liquidationFactorBps > 0 && _liquidationFactorBps <= 10000, "Invalid liquidation factor");
File: /src/Market.sol (line 162)
require(_replenishmentIncentiveBps > 0 && _replenishmentIncentiveBps < 10000, "Invalid replenishment incentive");
File: /src/Market.sol (line 173)
require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps + liquidationFeeBps < 10000, "Invalid liquidation incentive");
File: /src/Market.sol (line 184)
require(_liquidationFeeBps > 0 && _liquidationFeeBps + liquidationIncentiveBps < 10000, "Invalid liquidation fee");
File: /src/Market.sol (line 195)
require(recoveredAddress != address(0) && recoveredAddress == from, "INVALID_SIGNER");
File: /src/Market.sol (line 448)
require(recoveredAddress != address(0) && recoveredAddress == from, "INVALID_SIGNER");
File: /src/Market.sol (line 512)
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
function getWithdrawalLimitInternal(address user) internal returns (uint) {
File: /src/Market.sol (line 353)
#0 - c4-judge
2022-11-05T23:42:32Z
0xean marked the issue as grade-b