Platform: Code4rena
Start Date: 06/01/2023
Pot Size: $210,500 USDC
Total HM: 27
Participants: 73
Period: 14 days
Judge: 0xean
Total Solo HM: 18
Id: 203
League: ETH
Rank: 48/73
Findings: 1
Award: $121.59
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: CodingNameKiki
Also found by: 0xA5DF, 0xAgro, 0xNazgul, 0xSmartContract, Aymen0909, BRONZEDISC, Bnke0x0, Breeje, Cyfrin, GalloDaSballo, HollaDieWaldfee, IceBear, IllIllI, MyFDsYours, RaymondFam, Ruhum, SaharDevep, Sathish9098, Soosh, Udsen, __141345__, brgltd, btk, carlitox477, chaduke, chrisdior4, cryptonue, delfin454000, descharre, hihen, joestakey, ladboy233, lukris02, luxartvinsec, peanuts, pedr02b2, rotcivegaf, shark, tnevler, yongskiws
121.587 USDC - $121.59
During the audit, 2 low and 9 non-critical issues were found.
| â„– | Title | Risk Rating | Instance Count |
|---|---|---|---|
| L-1 | address signer != address(0) not checked | Low | 1 |
| L-2 | isContract condition | Low | 1 |
| NC-1 | Order of Functions | Non-Critical | 15 |
| NC-2 | Order of Layout | Non-Critical | 9 |
| NC-3 | Unused named return variables | Non-Critical | 13 |
| NC-4 | Open question | Non-Critical | 1 |
| NC-5 | Typos in function argument and variable names | Non-Critical | 6 |
| NC-6 | Typos | Non-Critical | 25 |
| NC-7 | Missing NatSpec | Non-Critical | 5 |
| NC-8 | One internal constant among public constants | Non-Critical | 1 |
| NC-9 | Missing leading underscores | Non-Critical | 67 |
address signer != address(0) not checkedWhen recover is invoked with an invalid signature, the zero-address is returned by it. With nonce == 0, the require(nonce == _useNonce(signer), "ERC20Votes: invalid nonce"); can be passed.
Check that address signer != address(0).
isContract conditionIt is unsafe to assume that an address for which isContract function returns false is an externally-owned account (EOA) and not a contract. It also can be a contract in construction, an address where a contract will be created, and an address where a contract lived, but was destroyed.
According to Style Guide, ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.
Functions should be grouped according to their visibility and ordered:
External functions should not be placed after internalc functions:
External functions should not be placed after public functions:
Public functions should not be placed after private/internal functions:
Reorder functions where possible.
According to Order of Layout, inside each contract, library or interface, use the following order:
Type declarations and state variables should be placed at the beginning of the contract, not between functions:
Events should be placed before all functions:
Modifiers should be placed before all functions:
Both named return variable(s) and return statement are used.
To improve clarity use only named return variables.
For example, change:
function functionName() returns (uint id) { return x;
to
function functionName() returns (uint id) { id = x;
Resolve the question.
bytes32 constant UIntOutofBoundsHash = keccak256(abi.encodeWithSignature("UIntOutOfBounds()")); => UIntOutOfBoundsHashfunction setFeeParameters(uint256 newFeeNumerator, address newfeeReceiverAddress) => newFeeReceiverAddressbytes memory returndata = address(token).functionCall( => returnData(bool success, bytes memory returndata) = target.call{ value: value }(data); => returnData(bool success, bytes memory returndata) = target.staticcall(data); => returnDatabytes memory returndata, => returnData/// Mointain the overall backing policy; handout assets otherwise => Maintain// (trades[addr] == true) iff this contract has created an ITrade clone at addr => if// lastPayoutBal' = rToken.balanceOf'(this) (balance now == at end of pay leriod) => period// queue.right == left iff there are no more pending issuances in this queue => if// issuances, and so any particular issuance is actually the _difference_ between two adjaacent => adjacent// The way to keep an IssueQueue striaght in your head is to think of each TotalIssue item as a => straight// We define a (partial) ordering on IssueItems: item1 < item2 iff the following all hold: => if* If this happens, users balances are zereod out and StRSR is re-issued at a 1:1 exchange rate => zeroed// r'.queue is r.queue with a new entry appeneded for (totalDrafts' - totalDraft) drafts => appended/// Overriden in StRSRVotes to handle rebases => Overridden// _delegates[account] is the address of the delegate that `accountt` has specified => account// Every function should revert iff its result is out of bounds. => if/// @return backing The worst-case collaterazation % the protocol will have after done trading => collateralization* @notice A UX-friendly layer for interactin with the protocol => interaction/// That is, id was cancelled iff firstId <= id < endId => if/// @param redeemer The address of the account redeeeming RTokens => redeeming/// Set the fraction of the RToken supply that can be reedemed at once => redeemed* Typically freezing thaws on its own in a predetemined number of blocks. => predetermined// trades[sell] != 0 iff trade[sell] has been opened and not yet settled => if* @return The amound of pending rewards in RAY => amount// @dev: function to intiate a new auction => initiate"limit price not better than mimimal offer" => minimal/// A Gnosis Mock that attemts to reenter on initiateAuction => attempts// from this trade's acution will all eventually go to origin. => auction// transfer all balancess of `buy` and `sell` at this address to `origin => balancesAdd NatSpec for all functions.
Here, all constants are public except for EIP712_DOMAIN:
bytes public constant EIP712_REVISION = bytes("1"); bytes32 internal constant EIP712_DOMAIN = keccak256( "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" ); bytes32 public constant PERMIT_TYPEHASH = keccak256( "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" ); bytes32 public constant METADEPOSIT_TYPEHASH = keccak256( "Deposit(address depositor,address recipient,uint256 value,uint16 referralCode,bool fromUnderlying,uint256 nonce,uint256 deadline)" ); bytes32 public constant METAWITHDRAWAL_TYPEHASH = keccak256( "Withdraw(address owner,address recipient,uint256 staticAmount,uint256 dynamicAmount,bool toUnderlying,uint256 nonce,uint256 deadline)" );
Make bytes32 internal constant EIP712_DOMAIN public.
Make the lines shorter.
Internal and private constants, variables and functions should have a leading underscore, public - should not:
Remove _ from public mapping:
Add _ to private/internal functions:
function handoutExcessAssets(IERC20[] calldata erc20s) private {function compromiseBasketsNeeded() private {function whenFinished(uint256 amtRToken) private returns (uint192 finished) {function refundSpan(function vestUpTo(address account, uint256 endId) private {function requireValidBUExchangeRate() private view {function requireNotPausedOrFrozen() private notPausedOrFrozen {}function pushDraft(address account, uint256 rsrAmount)function beginEra() internal virtual {function beginDraftEra() internal virtual {function rsrRewards() internal view returns (uint256) {function beginEra() internal override {function freezeUntil(uint48 newUnfreezeAt) private {function tryTrade(TradeRequest memory req) internal nonReentrant {function basketRange()function startedInSameEra(uint256 proposalId) private view returns (bool) {function processFeesAndAuctioneerFunds(function sendOutTokens(Add _ to private/internal constants/variables:
IBasketHandler private basketHandler;mapping(IERC20 => IAsset) private assets;IAssetRegistry private assetRegistry;IBasketHandler private basketHandler;IDistributor private distributor;IRToken private rToken;IERC20 private rsr;IStRSR private stRSR;IRevenueTrader private rsrTrader;IRevenueTrader private rTokenTrader;IBackingManager private backingManager;IRevenueTrader private rsrTrader;IRevenueTrader private rTokenTrader;mapping(address => bool) private trades;EnumerableSet.AddressSet internal destinations;IERC20 private rsr;IERC20 private rToken;address private furnace;address private stRSR;IRToken private rToken;IAssetRegistry private assetRegistry;IBasketHandler private basketHandler;IBackingManager private backingManager;IFurnace private furnace;uint192 private allVestAt; // D18{fractional block number}uint192 private lastIssRate; // D18{rTok/block}uint256 private lastIssRateBlock; // {block number}RedemptionBatteryLib.Battery private battery;mapping(IERC20 => uint256) private liabilities;IAssetRegistry private assetRegistry;IDistributor private distributor;IAssetRegistry private assetRegistry;IBackingManager private backingManager;IBasketHandler private basketHandler;IERC20 private rsr;mapping(uint256 => mapping(address => uint256)) private stakes; // Stakes per account {qStRSR}uint256 internal totalStakes; // Total of all stakes {qStRSR}uint256 internal stakeRSR; // Amount of RSR backing all stakes {qRSR}uint192 private constant MAX_STAKE_RATE = 1e9 * FIX_ONE; // 1e9 D18{qStRSR/qRSR}uint256 internal draftEra;uint256 internal totalDrafts; // Total of all drafts {qDrafts}uint256 internal draftRSR; // Amount of RSR backing all drafts {qRSR}uint192 private constant MAX_DRAFT_RATE = 1e9 * FIX_ONE; // 1e9 D18{qDrafts/qRSR}uint256 internal rsrRewardsAtLastPayout;IBroker private broker;mapping(uint256 => IterableOrderedOrderSet.Data) internal sellOrders;IdToAddressBiMap.Data private registeredUsers;mapping(address => bool) private trades;Add and remove leading underscores where needed.
#0 - c4-judge
2023-01-25T00:00:45Z
0xean marked the issue as grade-b