Category: Coding Mistakes
Users' funds can be stolen
Critical Severity
Critical Impact
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' shares
Impact
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↗.