Category: Coding Mistakes
Users' funds can be stolen
Critical Impact
Critical Severity
High Likelihood
Description
According to ERC-4626↗, the redeem function should deduct an allowance when msg.sender is not the owner, ensuring that only authorized users can redeem shares on behalf of others. However, in this contract, the redeem function does not enforce any such checks.
As a result, the function directly burns shares from the owner, regardless of whether msg.sender is authorized. This allows an attacker to call redeem with another user's owner address, burning their shares and redirecting assets to an arbitrary receiver.
function redeem(uint256 shares, address receiver, address owner) public override nonReentrant returns (uint256) {
uint256 maxShares = maxRedeem(owner);
require(shares > 0, "Cannot redeem 0");
require(shares <= maxShares, "Exceeded max redeem");
harvest();
uint256 assets = convertToAssets(shares);
uint256 _underlying = strategy.convertToAssets(assets);
_burn(owner, shares); // @audit does not burn caller's shares, can steal other users' sharesImpact
An attacker can redeem other users' shares.
Recommendations
Add a check to ensure the msg.sender is the owner or spends allowance from the owner.
Remediation
This issue has been acknowledged by StakeKit, and a fix was implemented in commit 22be2310↗.