Consistently check account balance before and after transfers for Fee-On-Transfer discrepancies

Lines of code Vulnerability details Impact Wrong amount calculated for facilitatorTake Proof of Concept Arbitrary ERC20 tokens can be passed as loanAssetContractAddress. With a transfer, the received amount should be calculated every time to take into consideration a possible fee-on-transfer or...

Loan buyouts can stick users with huge surprise bills

Lines of code Vulnerability details Impact If a small loan is bought out with a new loanAmount in the millions, so much interest can be charged that the borrower is unable to pay back the loan, essentially guaranteeing whales will be able to Seize NFTs. Proof of Concept The only requirement as it...

sendCollateralTo is unchecked in closeLoan(), which can cause user's collateral NFT to be frozen

Lines of code Vulnerability details function closeLoanuint256 loanId, address sendCollateralTo external override notClosedloanId requireIERC721borrowTicketContract.ownerOfloanId == msg.sender, "NFTLoanFacilitator: borrow ticket holder only"; Loan storage loan = loanInfoloanId;...

Loan can not be repaid when the BorrowTicket's owner is a contract with no onERC721Received method

Lines of code Vulnerability details In repayAndCloseLoan, after the loan is repaid, the Collateral NFT will safeTransferFrom addressthis to the owner of the BorrowTicket NFT, which is the mintBorrowTicketTo used in createLoan, must implement the onERC721Received method for the safeTransferFrom to...

requiredImprovementRate can not work as expected when previousInterestRate less than 10 due to precision loss

Lines of code Vulnerability details uint256 previousInterestRate = loan.perAnumInterestRate; uint256 previousDurationSeconds = loan.durationSeconds; requireinterestRate = previousDurationSeconds, 'NFTLoanFacilitator: duration too low'; requirepreviousLoanAmount requiredImprovementRate / SCALAR =...

currentLoanOwner can manipulate loanInfo when any lenders try to buyout

Lines of code Vulnerability details Impact If an attacker already calls lend to lend to a loan, the attacker can manipulate loanInfo by reentrancy attack when any lenders try to buyout. The attacker can set bad values of lendInfo e.g. very long duration, and 0 interest rate that the lender who...

Missing balance checking before giving approve to another address

Lines of code Vulnerability details Impact Approve function does not check whether msg.sender has enough balance equal or higher than amount parameter to approve it to spender address. So msg.sender can approve more than it's balance. Maybe it is not dangerous but checking balance is more safe...

NonCustodialPSM.mint ignores mint buffer reduced amount, mostly disabling mint rate control

Lines of code Vulnerability details Impact VOLT minting rate control doesn't happen when RateLimited's doPartialAction is true as NonCustodialPSM doesn't use the returned amount value, minting the full amount even when it was reduced by current mint buffer size limitation. There are two issues wi...

NonCustodialPSM.mint and redeem using mint control buffer in the inverted way

Lines of code Vulnerability details Impact Now there is no control of VOLT's issuance. For example, super fast VOLT mining is allowed, while mint control buffer will sit capped at its bufferCap, not affecting anything. This way mint speed control is disabled. The issue is that NonCustodialPSM.min...

Bad Incentives For Lenders Who Buy Out Other Lenders

Lines of code Vulnerability details Impact Lenders are allowed to "buy out" another lender on a position via the loan function. This is supposed to be a purely positive sum action for the borrower, as the new lender must provide "better" terms than their predecessor, as checked here: However, in...

NonCustodialPSM can become insolvent as CPI index rises

Lines of code Vulnerability details Impact NonCustodialPSM mints and redeems VOLT to a chosen stablecoin at the current market rate minus a fixed fee. It is assumed that the difference to be covered with pcvDeposit funds. That assumption is similar to one used in FEI protocol, but there no rate...

Unauthorized ERC20 changes

Lines of code Vulnerability details Impact It's possible by anyone to change the name and symbol of CToken. Proof of Concept The require was made in reverse condition, it checks that the sender must be different than admin in order to be able to change the name and symbol requiremsg.sender !=...

Possibility of stale and static value of VOLT for longer duration on change of ScalingPriceOracle

Lines of code Vulnerability details If there will be a need for governance action to swap out the ScalingPriceOracle that the OraclePassThrough points to, then it will have to be performed only after 15th till 30/31st of any month. This is due to two constraints viz, 1 28 days TIMEFRAME, and 2 th...

Borrowers lose funds if they call repayAndCloseLoan instead of closeLoan

Lines of code Vulnerability details Impact The repayAndCloseLoan function does not revert if there has not been a lender for a loan matched with lend. Users should use closeLoan in this case but the contract should disallow calling repayAndCloseLoan because users can lose funds. It performs a...

NonCustodialPSM's _getMintAmountOut and _getRedeemAmountOut don't account for underlyingToken decimals

Lines of code Vulnerability details Impact getMintAmountOut and getRedeemAmountOut are used to convert stablecoin amount to VOLT amount and back with the help of Oracle feed price. As stablecoin decimals differ both functions that do not scale the results accordingly will become grossly incorrect...

Need to change the logic of smart contracts in the face of negative inflation

Lines of code Vulnerability details Impact Given that the possibility of negative inflation in the economy is not impossible even now observed in some countries, it is important to clearly explain to users the act of smart contracts against negative inflation. In the current logic of the VOLT...

onlyBurner modifier missing

Lines of code Vulnerability details Impact onlyBurner modifier is missing in burn function. Since it is a public function anyone can burn FEI tokens. Proof of Concept Tools Used Manual analysis Recommended Mitigation Steps Add onlyBurner modifier to the burn function. --- The text was updated...

Can force borrower to pay huge interest

Lines of code Vulnerability details Impact The loan amount is used as a min loan amount. It can be matched as high as possible realistically up to the collateral NFT's worth to remain in profit and the borrower has to pay interest on the entire amount instead of just on the desired loan amount wh...

loan amount is unbounded

Lines of code Vulnerability details The lender of a loan can lend an unbounded amount, which the borrower might not be able to pay its interest rate. Proof of Concept Alice wants to borrow 1000 USDC, she creates a loan with a 10% interest rate, and expects to pay 100 USDC after a year. Bob lends...

Tokens with fee on transfer are not supported

Lines of code Vulnerability details There are ERC20 tokens that charge fee for every transfer / transferFrom. Vault.soladdValue assumes that the received amount is the same as the transfer amount, and uses it to calculate attributions, balance amounts, etc. But, the actual transferred amount can ...

Dangerous of one-step change of privileged roles

Lines of code Vulnerability details Impact Setting a new minter by old minter is happened in one step, and there is no way to correct it if old minter set a wrong address as new minter. Impact= high , likelihood= low -- Severity Risk = Medium owasp table Tools Used : manual analysis Recommended...

Lack of Limit on amount Allows Borrowers To Be Extorted For Interest Payments

Lines of code Vulnerability details Impact There is on limit on the upper bounds on the amount a lender may send to a borrower. Borrowers will then be forced to pay interest and facilitators fee on the full amount loaned. The impact for a borrower is that they may be forced to repay significantly...

Allowing the lender to finalize a loan with an arbitrarily high amount can cause the borrower to default

Lines of code Vulnerability details Impact A borrower specifies a minimum loan amount. The lender can provide a loan as high as they want. As long as it's higher than the minimum value. A value too high might cause the borrower to not be able to pay it back. A higher loan means a higher facilitat...

Faulty logic at _setBufferCap() function

Lines of code Vulnerability details Impact setBufferCap function sets the bufferCap. It first calls the updateBufferStored function which updates the bufferStored variable. However, calculation of bufferStored depends on bufferCap value: Math.minbufferStored + rateLimitPerSecond elapsed, bufferCa...

vcon address change not persistent across protocol components

Lines of code Vulnerability details Impact vcon address is allowed to be updated by GOVERNOR in Core, however, this change will not be reflected in CoreRef.vcon. Moreover, since CoreRef.vcon cannot be updated due to contract design, it is also impossible to fix this manually. We are not yet sure...

Re-entrace danger in PCVDeposit

Lines of code Vulnerability details Impact A malicious contract can empty all the system if he get ETH with the function withdrawETH. The onlyPCVController call this function and send ETH to the malicious contract, and then the malicious can use delegatecall and pretend to be onlyPCVController, a...

Involve governer to run this function

Lines of code Vulnerability details Impact Governer is an important role in revoking role decision, so it is recommended to involve it by using "onlyGoverner" modifier or using "onlyGovernerOrGaurdian" modifier in calling this function. Proof of Concept Tools Used : Manual analysis Recommended...

NFTLoanFacilitator: Unsupported fee-on-transfer tokens

Lines of code Vulnerability details Impact loanAssetContractAddress is specified by the user when the user creates a loan. When loanAssetContractAddress is fee-on-transfer tokens, in the lend function, the actual amount of tokens received by the contract will be less than the amount, so that the...

Inconsistent use of oracle

Lines of code Vulnerability details Impact Throughout the protocol, oracles are relied upon to keep Volt stable, calculate payouts to users, and judge whether actions are eligible to be carried out. On the NonCustodialPSM contract, oracle is updated with the updateOracle function. However, there ...

setMinter() in vcon.sol missing important checks can lead to loss of minting ability

Lines of code Vulnerability details Impact The setMinter function in Vcon.sol lacks both zero address checks and a proper ownership transfer pattern. I am submitting this as a medium-severity issue separate from similar low-severity instances due to this example's effect on the entire protocol. I...

Div by 0

Lines of code Vulnerability details Division by 0 can lead to accidentally revert, An example of a similar issue - code-423n4/2021-10-defiprotocol-findings84 https://github.com/code-423n4/2022-03-volt/tree/main/contracts/utils/Deviation.solL23 a might be 0 It's internal function but since it is...

NonCustodialPSM.mint double counts mint buffer depletion and GlobalRateLimitedMinter.mintVolt allows for minting in excess of the global mint buffer limit

Lines of code ttps://github.com/code-423n4/2022-03-volt/blob/main/contracts/utils/MultiRateLimited.solL333 Vulnerability details Impact Currently mint buffer control usage is incorrect, leading to either absence of mint speed control or to extensive control with amount partial double counting. Mi...

Might not get desired min loan amount if _originationFeeRate changes

Lines of code Vulnerability details Impact Admins can update the origination fee by calling updateOriginationFeeRate. Note that a borrower does not receive their minLoanAmount set in createLoan, they only receive 1 - originationFee minLoanAmount, see lend. Therefore, they need to precalculate the...

Wrong modfier

Lines of code Vulnerability details Impact According to the comment at line 49 and the business logic of this smart contract, it is better to use "ISgoverner " or "onlyGoverner" modifier instead of "onlyOwner". Proof of Concept Tools Used : manual analysis Recommended Mitigation Steps changing...

Missing balance check before permitting

Lines of code Vulnerability details Impact there is no owner balance checking before permit amount to spender. So owner can permit more than its balance to spender. Maybe this issue is not very risky but checking owner balance before permitting is more safe Proof of Concept Provide direct links t...

Facilitator fee is higher than it should be if loan asset has fees on transfer

Lines of code Vulnerability details Impact When the lend asset is a token with fees on transfer, the facilitator fee is higher than it should be. The contract doesn't use the actual token amount it received but the one where the fees aren't subtracted yet. This causes the borrower to receive fewe...

use of transferFrom

Lines of code Vulnerability details Impact NFTLoanFaciltator.closeLoan function uses transferFrom to transfer the collateral to "sendCollateralTo" address and if the contract and is not aware of incoming ERC721 tokens, the sent tokens could be locked forever Proof of Concept Tools Used Manual...

OracleRef assumes backup oracle uses the same normalizer as main oracle

Lines of code Vulnerability details Impact The OracleRef assumes that the backup oracle uses the same normalizer as the main oracle. This generally isn't the case as it could be a completely different oracle, not even operated by Chainlink. If the main oracle fails, the backup oracle could be...

Borrower may be forced to pay more interest than he expected

Lines of code Vulnerability details Impact Creating a loan with a minimum amount, instead of a fixed/maximum amount may lead to forcing the borrower to pay more interest than he was expecting or hoping for. The user never knows the potential maximum amount he will need to pay, until the loan is...

Updating rate limit for addresses restores their entire buffer amount

Lines of code Vulnerability details Impact When the bufferCap is updated for an address in updateAddress, the address's allowed buffer bufferStored is replenished to the entire bufferCap. The address could frontrun the updateAddress call and spend their entire buffer, then the buffer is replenish...

Lender can lose funds

Lines of code Vulnerability details Impact Since some tokens take transfer fees on performing transfer operations and current contract implementation is not considering same, lender funds could be lost Proof of Concept 1. Attacker creates a loan request for token XYZ taking 10% transfer fees 2...

Oracle price does not compound

Lines of code Vulnerability details Impact The oracle does not correctly compound the monthly APRs - it resets on fulfill. Note that the oraclePrice storage variable is only set in updateCPIData as part of the oracle fulfill callback. It's set to the old price price from 1 month ago plus the...

Chain ID Is Not Resistant To Hard Fork and Other Token Supports In The Oracle Contract

Lines of code Vulnerability details Impact During the code review, It has been observed only the following chain ids are supported for the chainlink. 1 and 42 - The contracts are not upgradeable therefore If there is any hard fork or new chain support, the contract should be deployed again with...

Setting new buffer does not reduce current buffer to cap

Lines of code Vulnerability details Impact The RateLimited.setBufferCap function first updates the buffer and then sets the new cap, but does not apply the new cap to the updated buffer. Meaning, the updated buffer value can be larger than the new buffer cap which should never be the case. Action...

Borrower collateral can be stolen

Lines of code Vulnerability details Impact In case of collateral price fluctuation, lender can create a situation where borrower would be unable to repay the loan and lender could get hold of collateral Proof of Concept 1. User creates a loan request for duration 1 day and minimum amount as 0.1...

NFTLoanFacilitator: Insufficient granularity allows for same-term loans to be accepted

Lines of code Vulnerability details Details & Impact It is possible for the calculated interest rate improvement to be zero if the existing interest rate is low enough ≤ 0.9% with 10% improvement rate. In such cases, lenders can compete to continually buyout each other with the same terms. The...

Royalties do Not Support Fee-On-Transfer Tokens

Lines of code Vulnerability details Impact The RoyaltyVault.sol contract interacts with the Splitter.sol to send accumulated royalties to the collection's respective recipients. The sendToSplitter function will query the balance of the royalty asset and send the amount after fee deductions to the...

Increasing the Lock Amount on an Expired Lock Will Cause Users to Miss Out on Rewards

Lines of code Vulnerability details Impact Paladin protocol allows users to increase the amount or duration of their lock while it is stil active. Increasing the amount of an active lock should only increase the total locked amount and it shouldn't make any changes to the associated bonus ratios ...

Users Can Bypass Emergency Restrictions on updateUserRewardState()

Lines of code Vulnerability details Impact The emergencyWithdraw function intends to withdraw their tokens regardless if they are locked up for any duration. This emergency must be triggered by the owner of the contract by calling triggerEmergencyWithdraw. A number of functions will revert when t...

Token Transfers Are Not Handled Correctly

Lines of code Vulnerability details Impact The transfer function is used to send royalty assets to the splitter contract and its recipients. If the vault operates on non-standard ERC20 tokens, its possible for transfers to not revert upon failure. Similarly, transferFrom is used to pull funds fro...