Underlying asset price oracle for CToken in BaseV1-periphery is inaccuarte

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Underlying asset price oracle for CToken in BaseV1-periphery is inaccuarte Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that...

The stable ctoken underlying price quote could be manipulated

Lines of code...

accountant and admin cannot be updated at Note.sol once accountant is initialized

Lines of code Vulnerability details Impact Once state variable accountant is set, accountant and admin will no longer be updated using setAccountantAddress function. Proof of Concept function setAccountantAddressaddress accountant external requiremsg.sender == admin; requireaddressaccountant ==...

Admin Can Broke All Functionality Through Weth Address

Lines of code Vulnerability details Impact On the protocol, almost all functionality is constructed through WETH address. however, If the admin is set to WETH address mistakenly, user could not claim through . Admin can break the protocol. Proof of Concept Tools Used Code Review Recommended...

Eth sent to Timelock will be locked in current implementation

Lines of code Vulnerability details Impact Eth sent to Timelock will be locked in current implementation. I came across this problem while playing around with the governance contract. Proof of Concept Setup the governance contracts GovernorBravoDelegate, Timelock Send eth to timelock contract Set...

Unused deadline checker modifier may result in user fund loss due to unexpect transaction ordering (MEV)

Lines of code Vulne...

_initiate() function can be called multiple times at GovernorBravoDelegate.sol

Lines of code Vulnerability details Impact initiate function is supposed to be called only once, but it can be called many times which are not expected behavior. Proof of Concept function initiate external requiremsg.sender == admin, "GovernorBravo::initiate: admin only"; requireinitialProposalId...

Test

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The...

Contract TresureDelegate.sol could be destructed

Lines of code Vulnerability details Impact The contract simply could be destructed by anyone. Proof of Concept The question is how?! Imagine, that the recipient account has a fallback function with a selfdestruct in it. ./attack.sol contract attack fallback external payable...

Frequent price update make the project vulnerable to price oracle manipulation

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Frequent price update make the project vulnerable to price oracle manipulation Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof...

The LP pair underlying price quote could be manipulated

Lines of code Vulnerability details The LP pair underlying price quote could be manipulated Impact The underlying price for LP pool pair can be manipulated. This kind of price mainpulation happened before, can be found here: Warp Fincance event. Whick may lead to the exploit of the pool by a...

Attackers can call update and grief users from swapping tokens or minting/burning because those functions divide by zero

Lines of code Vulnerability details Details An attacker can specify reserve0 and reserve1 in update function and make them both zero. Every function using those variable to divide would revert causing a dos and causing lost of gas for users proof of concept Attacker has a contract that just keeps...

Total Cash Prior Is not Considered On the Exchange Rate

Lines of code Vulnerability details Impact Cash prior is not considered on the exchangeRateStoredInternal calculation. Even if Its declared as "value can be zero", the getCashPrior function is still defined on the contract. Proof of Concept Tools Used Code Review Recommended Mitigation Steps...

Users may lose funds due to ignored return value of transfer()

Lines of code Vulnerability details Impact The return value of WETH transfer in is unchecked. So if the transfer fails, it continues to L1386 which returns 0 to compAccruedholdersj in L1370 there by resetting the compAccrued of the holder to zero. With this, the user would lose funds. Proof of...

getSupplyRate is returning baseRatePerYear instead of baseRatePerBlock

Lines of code Vulnerability details Impact getSupplyRate must return rate per block as defined in compound interest rate model but currently return rate per year. Defined here: Proof of Concept This is definition / @notice Calculates the current supply interest rate per block @param cash The tota...

QA Report

ensure modifier - require statement is commented out The ensureuint deadline modifier is used in multiple functions such as addLiquidity and removeLiquidity but the require statement is commented out, the modifier is essentially usesless. modifier ensureuint deadline //requiredeadline =...

Miners can influence the value of block.timestamp to perform Maximal Extractable Value (MEV) attacks.

Lines of code Vulnerability details Impact Miners can influence the value of block.timestamp to perform Maximal Extractable Value MEV attacks. The use of now creates a risk that time manipulation can be performed to manipulate price oracles. Miners can modify the timestamp by up to 900 seconds...

period size is incorrectly set

Lines of code Vulnerability details Impact Instead of setting periodSize to 1800 30 min, it is currently set to 0 which means new observations will get added each time update is called at BaseV1-core.solL166 Proof of Concept 1. Observe that periodSize is set to 0 2. BaseV1-core.solL166 will alway...

Sending 1 wei of cNote to the treasury can malfunction sweepInterest()

Lines of code Vulnerability details function sweepInterest external override if msg.sender != admin revert SenderNotAdminmsg.sender; //Total balance of Treasury = Note + CNote Balance, Exp memory exRate = Expmantissa: cnote.exchangeRateStored; //used stored interest rates in determining amount to...

GovernorBravoDelegate.sol _initiate() can be called multiple times

Lines of code Vulnerability details Submitting as med risk because documentation inside functions shows that require is there for the purpose of preventing it being called again but is not working correctly, and since we do not have access to the timelock code I cannot fully assess the impact...

Stableswap - Deadline do not work

Lines of code Vulnerability details Impact The ensure modifier is commented, so deadlines will not work when passing orders, breaking this functionnality. --- The text was updated successfully, but these errors were encountered: All reactions...

Non view function is called with staticcall in CErc20Delegator

Lines of code Vulnerability details Impact When using CToken implementation with CErc20Delegator, the functions borrowRatePerBlock and supplyRatePerBlock will revert when the underlying functions try to update some states. Detail The v1 of borrowRatePerBlock and supplyRatePerBlock were view...

A cap is needed on the amount of Note than can be borrowed

Lines of code Vulnerability details Impact The fact that there is no cap on the amount of Note that can be borrowed makes the Oracle Extractable Value unlimited. But as you intend to rely on TWAP, you need to make sure the cost of oracle manipulation is lower than the Oracle Extractable Value...

updateBaseRate and getBorrowRate will always revert if Note/gUSDC TWAP is above 1

Lines of code Vulnerability details Impact updateBaseRate will revert if Note/gUSDC TWAP is above 1 causing all dependent functions to revert as well Proof of Concept If twapMantissa is greater than 1 then L147 will revert due to underflow error Tools Used Recommended Mitigation Steps It seems th...

Upgraded Q -> M from 18 [1656705908645]

Judge has assessed an item in Issue 18 as Medium risk. The relevant finding follows: Controlled swapRouter The FeeBurner contract sets the swapperRouter in the addressProvider, so the owner can set any type of swapper, paths or pools, even malicious ones. Since there is no slippage defined in the...

Upgraded Q -> M from 18 [1656705895450]

Judge has assessed an item in Issue 18 as Medium risk. The relevant finding follows: Multiple initialization The initialize method of the BkdLocker contract allows it to be started multiple times as long as the value startBoost=0 is set. Abuse these settings to his advantage. Affected source code...

getBorrowRate must return rate per block but currently return rate per year

Lines of code Vulnerability details Impact getBorrowRate must return rate per block as defined in compound interest rate model but currently return rate per year. Defined here: Proof of Concept This is definition / @notice Calculates the current borrow interest rate per block @param cash The tota...

Oracle periodSize is very low allowing the TWAP price to be easily manipulated

Lines of code Vulnerability details Impact TWAP oracle easily manipulated Proof of Concept periodSize is set to 0 meaning that the oracle will take a new observation every single block, which would allow an attacker to easily flood the TWAP oracle and manipulate the price Tools Used Recommended...

AccountantDelegate.sol#sweepInterest() Extra amount of cNote will sent to the treasury, causing a shortage of cNote on the accountant's account

Lines of code Vulnerability details function sweepInterest external override if msg.sender != admin revert SenderNotAdminmsg.sender; //Total balance of Treasury = Note + CNote Balance, Exp memory exRate = Expmantissa: cnote.exchangeRateStored; //used stored interest rates in determining amount to...

Total supply can be incorrect in ERC20

Lines of code Vulnerability details Impact totalSupply can be initialized to something different than 0, which would lead to an inaccurate total supply, and could easily break integrations, computations of market cap, etc. Proof of Concept If the constructor is called with initialSupply = 1000, t...

Multiple functions in GovernorBravoDelegator.sol could cause dangerous future mistakes

Lines of code Vulnerability details Submitting as med risk because these are very important functions and using requires like this seems very likely to cause future mistakes Impact Increases likelihood of future vulnerabilities Proof of Concept initiateDelegated acceptInitialAdminDelegated and...

User can claim double comp in one function call

Lines of code Vulnerability details Impact Comptroller.seizeAllowed and Comptroller.transferAllowed are functions that call distributeSupplierComp twice without ensuring the address for the supplier param are not the same. So when seizeAllowed with borrower and liquidator as same user address or...

Failed transfer with low level call won't revert

Lines of code Vulnerability details In BaseV1-periphery.sol, the function safeTransferCANTO is as follows: function safeTransferCANTOaddress to, uint value internal bool success, = to.callvalue:valuenew bytes0; requiresuccess, "TransferHelper: ETHTRANSFERFAILED"; This function is utilized in a fe...

lack of validating transfer of tokens entered by the user

Lines of code Vulnerability details Impact As we have not defined the implementation of note, it should be validated that note.transferrecipient, amount returns true. This is important, since the transaction could not be carried out and the function executed as correct. In the comments of the cod...

lack of validating transfer of tokens entered by the user or by the initialize() function

Lines of code Vulnerability details Impact As we have not defined the token implementation, it should be validated that token.transferadmin, balance; return true. This is important, since the transaction could not be carried out and the function executed as correct. In the comments of the code it...

Upgraded G -> M from 49 [1656600225781]

Judge has assessed an item in Issue 49 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

lack of validating transferFrom of token

Lines of code Vulnerability details Impact As we have not defined the token implementation, it should be validated that token.transferFromfrom, addressthis, amount; return true. This is important, since the transaction could not be carried out and the function executed as correct. In the comments...

baseRatePerBlock not updated when a new base rate is set

Lines of code Vulnerability details Impact When an admin sets a new baseRatePerYear in setBaseRatePerYear, the baseRatePerBlock is not updated. If the deltaBlocks has not passed yet, it will also not be updated when getSupplyRate is called, i.e. a stale value will be returned there. Recommended...

getSupplyRate returns rate per year instead of per block

Lines of code Vulnerability details Impact According to the documentation in InterestRateModel, getSupplyRate has to return the supply rate per block and the function supplyRatePerBlock in CToken directly returns the value of getSupplyRate. However, the rate per year is returned for NoteInterest...

Multiple initialization in NoteInterest

Lines of code Vulnerability details Impact The initialize method of the contract NoteInterest can be initialized multiple times. Proof of Concept The method initialize of the contract NoteInterest looks like this: function initializeaddress cnoteAddr, address oracleAddress external if msg.sender ...

AccountantDelegate: The sweepInterest function sweeps an incorrect number of cnote.

Lines of code Vulnerability details Impact In the sweepInterest function of the AccountantDelegate contract, the number of cnote sent to treasury should be cNoteToSweep instead of amtToSweep, as amtToSweep will normally be smaller than cNoteToSweep, which will cause the interest to be locked in t...

Oracle periodSize = 0 which is as same as not using any oracle.

Lines of code Vulnerability details Impact Oracle periodSize = 0 which is as same as not using any oracle. It should be 1800 as expected 30 minutes. Proof of Concept // Capture oracle reading every 30 minutes uint constant periodSize = 0; You comment said that capture oracle reading every 30...

getBorrowRate returns rate per year instead of per block

Lines of code Vulnerability details Impact According to the documentation in InterestRateModel, getBorrowRate has to return the borrow rate per block and the function borrowRatePerBlock in CToken directly returns the value of getBorrowRate. However, the rate per year is returned for NoteInterest...

An attacker can render CNote’s doTransferOut unusable

Lines of code Vulnerability details Impact In doTransferOut, the underlying balance of the CNote has to be 0 after the transfer. While this works fine when the underlying balance of the CNote was 0 before the call i.e., in the normal case, the function will always revert when the balance was...

redeemFresh does not work

Lines of code Vulnerability details Impact The CNote contract uses the CToken contract's redeemFresh function to perform redeem operations. However, since the CNote contract does not hold note tokens, the getCashPrior returns 0 and is less than the redeemAmount, causing the redeemFresh function t...

No authentication for SimplePriceOracle

Lines of code Vulnerability details Impact Anyone can call setUnderlyingPrice on the SimplePriceOracle to set the oracle values. These are in turn used for the interest calculations, meaning anyone can manipulate this calculation via the Oracle. Note that SimplePriceOracle in Compound is only...

Centralized mint

Lines of code Vulnerability details Impact The mint method is too centralized. Proof of Concept Using the mint function of ERC20MinterBurnerDecimals, the minter address can mint arbitrary amount of tokens. If the private key of the owner or minter address is compromised, the attacker will be able...

BURNER_ROLE can burn any amount of ERC20MinterBurnerDecimals from an arbitrary address

Lines of code Vulnerability details Vulnerability details Proof of Concept Using the burnCoins function of ERC20MinterBurnerDecimals, an address with BURNERROLE can burn an arbitrary amount of tokens from any address. We believe this is unnecessary and poses a serious centralization risk. A...

Deny of service in AccountantDelegate.sweepInterest

Lines of code Vulnerability details Impact The sweepInterest method is susceptible to denial of service. Proof of Concept The logic of the sweepInterest method relative to the treasury is as follows: bool success = cnote.transfertreasury, amtToSweep; if !success revert SweepErrortreasury ,...

Deny of service in CNote.doTransferOut

Lines of code Vulnerability details Impact The CNote.doTransferOut method is susceptible to denial of service. Proof of Concept The logic of the doTransferOut method in CNote is as follows: function doTransferOutaddress payable to, uint amount virtual override internal requireaddressaccountant !=...