Platform: Code4rena
Start Date: 28/06/2022
Pot Size: $25,000 USDC
Total HM: 14
Participants: 50
Period: 4 days
Judge: GalloDaSballo
Total Solo HM: 7
Id: 141
League: ETH
Rank: 32/50
Findings: 1
Award: $63.37
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: zzzitron
Also found by: 0v3rf10w, 0x1f8b, 0x29A, AlleyCat, Bnke0x0, Chom, Funen, JC, Lambda, Limbooo, Meera, Picodes, Sm4rty, TerrierLover, TomJ, __141345__, asutorufos, aysha, c3phas, cccz, defsec, fatherOfBlocks, grGred, hake, ignacio, ladboy233, mrpathfindr, oyc_109, rfa, sach1r0, samruna, slywaters, ynnad
63.3666 USDC - $63.37
Title: Insufficient Input Validation
Existing Id could cause AddProposal function to revert.
Check if the proposal ID already exists or not. https://github.com/Plex-Engineer/manifest-v2/blob/f6ebfe679973edf4f64832e64480ff5250ef8486/contracts/Proposal-Store.sol#L42
Manual
Check if the id exists already or not. Check if the proposal was created successfully.
Title: Unlocked pragma
Contracts should be deployed using the same compiler version/flags with which they have been tested.
see here: https://swcregistry.io/docs/SWC-103 pragma solidity ^0.8.10 https://github.com/Plex-Engineer/manifest-v2/blob/f6ebfe679973edf4f64832e64480ff5250ef8486/contracts/Proposal-Store.sol#L3 …
Manual
Locking the pragma (for e.g. by not using ^ in pragma solidity 0.8.10) ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs.
Title: Insufficient Input Validation
The functions should first check if the passed arguments are valid first.
External functions that does not check the input values: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/BaseJumpRateModelV2.sol#L67
Manual
Check input values
Title: Missing Checks For Approve()’s Return status
Check if this function is set correctly.
Manual
Use require() or require or OpenZeppelin’s safeApprove()
Title: Missing Checks For approve()’s Return status
Check if this function is set correctly.
Manual
Use require() or require or OpenZeppelin’s safeIncreaseAllowance()/safeDecreaseAllowance()
Title: Missing Checks For transferFrom()’s Return status
Check if this function transfers assets correctly.
Manual
Use require() for dai And require or OpenZeppelin’s safeTransferFrom() for note
Title: Missing Checks For transfer()’s Return status
Check if this function transfers assets correctly.
Manual
Use require() or require or OpenZeppelin’s safeTransfer()
Title: Unsafe Casts And Usage Of IERC20Detailed
not all ERC20 contracts define decimals() since it’s optional in the spec
https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L118 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L119
Manual
Use safeDecimals() instead decimals0 = 10IERC20Detailed(_token0).decimals(); decimals1 = 10IERC20Detailed(_token1).decimals();
Title: Event is missing indexed fields
Each event should use three indexed fields if there are three or more fields
lending: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L10 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Comptroller.sol#L19-L76 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/ComptrollerG7.sol#L18-L66 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CTokenInterfaces.sol#L128-L201 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/ErrorReporter.sol#L53 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/JumpRateModel.sol#L11 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L64-L85 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/SimplePriceOracle.sol#L9 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Timelock.sol#L12-L14 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Unitroller.sol#L16-L31 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/WhitePaperInterestRateModel.sol#L12 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Accountant/AccountantInterfaces.sol#L24-L26 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/Comp.sol#L48-L57 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorAlpha.sol#L116-L128 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Governance/GovernorBravoInterfaces.sol#L9-L30 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Stableswap/BaseV1-core.sol#L100-L104
Manual
Add Index to at least 3 parameters or existing ones in fewer cases.
Title: Two-step change of privileged roles
In a single-step change, if the current admin accidentally changes the new admin to a zero-address or an incorrect address (where the private keys are not available), the system is left without an operational admin and will have to be redeployed.
https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/NoteInterest.sol#L109 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Note.sol#L21
Manual
When privileged roles are being changed, it is recommended to follow a two-step approach: 1) The current privileged role proposes a new address for the change 2) The newly proposed address then claims the privileged role in a separate transaction. This two-step change allows accidental proposals to be corrected instead of leaving the system operationally with no/malicious privileged role.
#0 - GalloDaSballo
2022-08-13T22:09:45Z
Valid Low
NC
Valid Low
Disputed as implementation is known, and both tokens always return true (and revert on error)
https://etherscan.io/address/0x6b175474e89094c44da98b954eedeac495271d0f#code#L156
Disputing per the reason above, and ignoring the ETH one
Valid low
Disagree with rest
I feel adding some personality would help a lot as this feels like a scrape of old reports.
Also recommend fixing formatting, the title, which should be a heading, is the only line which is not a heading
#1 - GalloDaSballo
2022-08-13T22:09:54Z
3L 1NC