Summary
An attacker can launch a DoS attack by calling the distributeYield() function with a huge array of address[] calldata assets, making the protocol service unavailable for other users.
Vulnerability Details
distributeYield() allows the array assets to be passed as a param with no limit on its length. Also, duplicate values inside the array are allowed.
File: contracts/facets/YieldFacet.sol
53 function distributeYield(address[] calldata assets) external nonReentrant {
54 uint256 length = assets.length;
55 uint256 vault = s.asset[assets[0]].vault;
56
57
58 (uint88 yield, uint256 dittoYieldShares) = _distributeYield(assets[0]);
59
60
61 for (uint256 i = 1; i < length;) {
62 if (s.asset[assets[i]].vault != vault) revert Errors.DifferentVaults();
63 (uint88 amtYield, uint256 amtDittoYieldShares) = _distributeYield(assets[i]);
64 yield += amtYield;
65 dittoYieldShares += amtDittoYieldShares;
66 unchecked {
67 ++i;
68 }
69 }
70
71 _claimYield(vault, yield, dittoYieldShares);
72 emit Events.DistributeYield(vault, msg.sender, yield, dittoYieldShares);
73 }
So, an attacker can create a huge array with a single asset address at every index and pass it to the function. This causes a DoS attack and can cause the protocol service to be unavailable to other users. Note that there is nothing to be gained in terms of yield by passing an array with duplicate asset addresses, so this would be done only for a DoS attack.
Add the following test inside test/Yield.t.sol and run it through forge test --mt test_DistributeYield_HugeDuplicateAssetArray -vv.
function test_DistributeYield_HugeDuplicateAssetArray() public {
fundLimitShortOpt(DEFAULT_PRICE, DEFAULT_AMOUNT, receiver);
fundLimitBidOpt(DEFAULT_PRICE, DEFAULT_AMOUNT, sender);
skip(yieldEligibleTime);
generateYield(1 ether);
address[] memory assets = new address[](4);
assets[0] = asset;
assets[1] = asset;
assets[2] = asset;
assets[3] = asset;
uint256 ethEscrowed1 = diamond.getVaultUserStruct(vault, receiver).ethEscrowed;
vm.prank(receiver);
diamond.distributeYield(assets);
uint256 ethEscrowed2 = diamond.getVaultUserStruct(vault, receiver).ethEscrowed;
assertApproxEqAbs(ethEscrowed2 - ethEscrowed1, 900000000000000000, MAX_DELTA);
}
Impact
DoS attack makes the protocol service unavailable for other users.
Tools Used
Manual inspection and foundry.
Recommendations
A constraint like the following can be added in distributeYield() which puts an upper limit on the assets array size being passed into the function. If a user really needs to pass many assets, he can try in separate transactions.
53 function distributeYield(address[] calldata assets) external nonReentrant {
54 uint256 length = assets.length;
+ require(length < 10, "Too many assets");
55 uint256 vault = s.asset[assets[0]].vault;
56
57 // distribute yield for the first order book
58 (uint88 yield, uint256 dittoYieldShares) = _distributeYield(assets[0]);
59
60 // distribute yield for remaining order books
61 for (uint256 i = 1; i < length;) {
62 if (s.asset[assets[i]].vault != vault) revert Errors.DifferentVaults();
63 (uint88 amtYield, uint256 amtDittoYieldShares) = _distributeYield(assets[i]);
64 yield += amtYield;
65 dittoYieldShares += amtDittoYieldShares;
66 unchecked {
67 ++i;
68 }
69 }
70 // claim all distributed yield
71 _claimYield(vault, yield, dittoYieldShares);
72 emit Events.DistributeYield(vault, msg.sender, yield, dittoYieldShares);
73 }
