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

In `Function:deposit`, `stakedAsset[receiver]` uses `=` to update insteadof `+=`, incorrectly updating the state of `stakedAsset[receiver]`

zhongguangyang0907

[H-2] In Function:deposit, stakedAsset[receiver] uses = to update insteadof +=, incorrectly updating the state of stakedAsset[receiver]

Description

In Function:deposit, stakedAsset[receiver] uses = to update insteadof +=:

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
......
​
@> stakedAsset[receiver] = stakeAsset;
​
......
​
}

Impact

Assume there is no fee.
If userA first deposits 5 ether into the contract, and then deposits another 1 ether, stakedAsset[userA] now equals 1 ether instead of 6 ether.

Proof of Concepts

Run the test funtion below in test/briVault.t.sol:

function test_incorrect_stakedAsset_update() public {
uint256 base = 10000;
uint256 expected_stakedAsset;
vm.startPrank(user1);
​
//user1 first deposit 5 ehter.
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
expected_stakedAsset = 5 ether - (5 ether * participationFeeBsp) / base;
assertEq(briVault.stakedAsset(user1), expected_stakedAsset);
​
//user1 then deposit another 5 ether.
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user1);
expected_stakedAsset += 5 ether - (5 ether * participationFeeBsp) / base;
​
//find that actual stakedAsset less than expected_stakedAsset.
assertLt(briVault.stakedAsset(user1), expected_stakedAsset);
console.log("actual_stakedAsset", briVault.stakedAsset(user1));
}

Recommended mitigation

function deposit(uint256 assets, address receiver) public override returns (uint256) {
require(receiver != address(0));
if (block.timestamp >= eventStartDate) {
revert eventStarted();
}
​
......
​
- stakedAsset[receiver] = stakeAsset;
+ stakedAsset[receiver] += stakeAsset;
......
​
}
Updates

Appeal created

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

`stakedAsset` Overwritten on Multiple Deposits

Vault tracks only a single deposit slot per user and overwrites it on every call instead of accumulating the total.

Support

FAQs

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

Give us feedback!