Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

A malicious user can DoS the removal or update of strategies

dimah7

Summary

When the owner decides to remove a strategy or update strategies the internal StakingPool::_updateStrategyRewards() is called to distribute rewards/fees based on changes in balance since last distribution. However a malicious user can DoS the process.

Vulnerability Details

Looking at the _updateStrategyRewards():

function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
...
if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
_mintShares(address(this), sharesToMint);
​
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
if (feesPaidCount == totalFeeCount - 1) {
@> transferAndCallFrom(
address(this),
receivers[i][j],
balanceOf(address(this)),
"0x"
);
} else {
@> transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
}
...
}

We can see that ERC677's transferAndCallFrom() is called to handle token transfers, which also triggers the callback function onTokenTransfer() if the recipient is a smart contract:

function transferAndCallFrom(
address _sender,
address _to,
uint256 _value,
bytes memory _data
) internal returns (bool) {
_transfer(_sender, _to, _value);
if (isContract(_to)) {
contractFallback(_sender, _to, _value, _data);
}
return true;
}
​
function contractFallback(
address _sender,
address _to,
uint256 _value,
bytes memory _data
) internal {
IERC677Receiver receiver = IERC677Receiver(_to);
receiver.onTokenTransfer(_sender, _value, _data);
}

However if a maliciously constructed smart contract implements the onTokenTransfer() to revert when it's triggered the whole distribution will be undone, hence the update or removal of strategy will also be prevented DoS-ing the protocol owner. Here is the mentioned code:

function onTokenTransfer(address, uint256, bytes memory) public {
revert();
}

Impact

A malicious user can DoS the protocol owner, which stops the essential workflow of the project, because he will not be able to update or remove a strategy when it's underperforming for example.

Tools Used

Manual Review

Recommendations

I would suggest to implement pull-over-push mechanism or wrap the callback function in a try/catch block to handle such situations

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

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

Inability to independently update Fee receivers without sending fees to old receivers

Support

FAQs

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