Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Attacker can poison user's `depositIdList` by transferring 1 wei Silo deposits to DOS withdrawal

T1MOH

Summary

When user receives new Silo deposit (by depositing, or receives via transfer), this depositId is added to array depositIdList.

When user withdraws his deposit, depositId is removed from that depositIdList. Array is looped through to find id to remove.

Problem is that griefer can transfer a bunch of 1 wei Silo deposits to make user's array depositId too big to loop on removal. As a result user can't execute full withdrawal of his deposit. Issue becomes severe when user is integrator's contract with limited functinality, i.e. it can't perform partial withdrawal.

Vulnerability Details

On transfers and deposits function addDepositToAccount() is called where new depositId is pushed to array:

function addDepositToAccount(
address account,
address token,
int96 stem,
uint256 amount,
uint256 bdv,
Transfer transferType
) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256 depositId = LibBytes.packAddressAndStem(token, stem);
​
// add a depositId to an account's depositList, if there is not an existing deposit.
if (s.accts[account].deposits[depositId].amount == 0) {
@> s.accts[account].depositIdList[token].push(depositId);
}
...
}
​

On full withdrawal it tries to remove depositId from depositIdList by looping through array:

function removeDepositFromAccount(
address account,
address token,
int96 stem,
uint256 amount
) internal returns (uint256 crateBDV) {
...
// Full remove
if (crateAmount > 0) {
delete s.accts[account].deposits[depositId];
@> removeDepositIDfromAccountList(account, token, depositId);
}
​
...
}
function removeDepositIDfromAccountList(
address account,
address token,
uint256 depositId
) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
@> uint256 i = findDepositIdForAccount(account, token, depositId);
...
}
function findDepositIdForAccount(
address account,
address token,
uint256 depositId
) internal view returns (uint256 i) {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256[] memory depositIdList = s.accts[account].depositIdList[token];
uint256 length = depositIdList.length;
while (depositIdList[i] != depositId) {
i++;
if (i >= length) {
revert("Id not found");
}
}
return i;
}

Impact

User can be unable to perform full withdrawal of Silo deposit. That's because depositIdList is looped through on full withdrawal which reverts with OOG error.

Tools Used

Manual Review

Recommendations

There is no way to mitigate this issue, possible solution is to not remove depositId from depositIdList

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Zero transfers (or 1 wei) and deposits in Silo corrupt array `depositIdList`

Appeal created

T1MOH Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Zero transfers (or 1 wei) and deposits in Silo corrupt array `depositIdList`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.