BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Receiver Parameter Mismatch in deposit() Function

objectplayer

Root + Impact

Shares minted to wrong address, breaking deposit-on-behalf functionality.

Description

  • Function should either use receiver for all operations or not at all.

  • Credits stakedAsset to receiver but mints shares to msg.sender.

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
stakedAsset[receiver] = stakeAsset; // @> Credits receiver
_mint(msg.sender, participantShares); // @> Mints to msg.sender!
}

Risk

Likelihood:

  • Occurs when receiver != msg.sender

  • Common pattern in ERC4626

  • Breaks expected behavior

Impact:

  • Receiver has credited stake but no shares

  • msg.sender has shares but no stakedAsset

  • Neither can join event

  • Funds effectively locked

Proof of Concept

Here is a PoC of calling deposit method from alice and sending bob as a receiver

alice.deposit(1000, bob);
​
// stakedAsset[bob] = 1000
// balanceOf(bob) = 0
// balanceOf(alice) = shares
​
// Bob can't join (no shares)
bob.joinEvent(0); // REVERT
​
// Alice can't join (no stakedAsset)
alice.joinEvent(0); // REVERT
​
// Funds locked!

Recommended Mitigation

There can be multiple mitigations, like removing of receiver address and all the states update for the msg.sender, another mitigation will be using receiver address and all states updates for the receiver

Mitigation-1: Using of msg.sender

function deposit(uint256 assets) public override returns (uint256) {
- require(receiver != address(0));
​
uint256 stakeAsset = assets - fee;
​
- stakedAsset[receiver] = stakeAsset;
+ stakedAsset[msg.sender] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
​
IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
​
_mint(msg.sender, participantShares);
​
- emit deposited (receiver, stakeAsset);
+ emit deposited (msg.sender, stakeAsset);
​
return participantShares;
}

Mitigation-2: Using of receiver

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
​
uint256 stakeAsset = assets - fee;
​
stakedAsset[receiver] = stakeAsset;
​
uint256 participantShares = _convertToShares(stakeAsset);
​
- IERC20(asset()).safeTransferFrom(msg.sender, participationFeeAddress, fee);
+ IERC20(asset()).safeTransferFrom(receiver, participationFeeAddress, fee);
​
- IERC20(asset()).safeTransferFrom(msg.sender, address(this), stakeAsset);
+ IERC20(asset()).safeTransferFrom(receiver, address(this), stakeAsset);
​
- _mint(msg.sender, participantShares);
+ _mint(receiver, participantShares);
​
emit deposited (receiver, stakeAsset);
​
return participantShares;
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Shares Minted to msg.sender Instead of Specified Receiver

Support

FAQs

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

Give us feedback!