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

Zero transfers and deposits in Silo corrupt array `depositIdList`

T1MOH

Summary

Identical depositIds can be added to array of depositIds depositIdList during zero transfers. And as a result storage will be corrupted because they cannot be easily removed.
It will result in view functions returning incorrect value.

Vulnerability Details

This Gist contains test showing that identical depositIds are added into array on zero transfers
https://gist.github.com/T1MOH593/00c80121e0cf98de1c39499f035b4161

Here you can see that removal removes only first occurence in array:

function removeDepositIDfromAccountList(
address account,
address token,
uint256 depositId
) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
@> uint256 i = findDepositIdForAccount(account, token, depositId);
s.accts[account].depositIdList[token][i] = s.accts[account].depositIdList[token][
s.accts[account].depositIdList[token].length - 1
];
s.accts[account].depositIdList[token].pop();
}
​
/**
* @notice given an depositId, find the index of the depositId in the account's deposit list.
*/
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

Array depositIdList can be poisoned with 0 deposits, moreover it will contain identical multiple depositIds. And those identical depositIds are incorrectly removed.

Tools Used

Manual Review

Recommendations

Don't add 0 deposits 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) {
+ if (s.accts[account].deposits[depositId].amount == 0 && amount != 0) {
s.accts[account].depositIdList[token].push(depositId);
}
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`

Support

FAQs

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