Due to the lack of access control it is possible for a malicous actor to reset a users reward acrrual, meaning that when a user tried to claim said rewards they could be less than anticipated without the user knowing why or how
Proof of Concept
function userAccrue(ERC20 producerToken, address user) public {
if (address(producerToken) == address(0)) revert ZeroAddress();
if (user == address(0)) revert ZeroAddress();
UserState storage u = producerTokens[producerToken].userStates[user];
uint256 balance = producerToken.balanceOf(user);
// Calculate the amount of rewards accrued by the user up to this call
uint256 rewards = u.rewards +
u.lastBalance *
(block.timestamp - u.lastUpdate);
u.lastUpdate = block.timestamp.safeCastTo32();
u.lastBalance = balance.safeCastTo224();
u.rewards = rewards;
emit UserAccrue(producerToken, user, block.timestamp, balance, rewards);
}
The above function can be called currently by anyone, users address is an arbiturary value the currently anyone can enter, if a malicious character has the address of a user who is interacting with this protocol, due to the following calculations
A malicious actor can call this function on behalf of a given user and reset the reward accrual by reseting the u.lastUpdate and therefor restarting a reward accrual proccess. This particular user now has less u.rewards than they should
Tools Used
Recommended Mitigation Steps
To mitigate add some kind of access control to require(user == msg.sender)
Lines of code
https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexRewards.sol#L281
Vulnerability details
Impact
Due to the lack of access control it is possible for a malicous actor to reset a users reward acrrual, meaning that when a user tried to claim said rewards they could be less than anticipated without the user knowing why or how
Proof of Concept
The above function can be called currently by anyone, users address is an arbiturary value the currently anyone can enter, if a malicious character has the address of a user who is interacting with this protocol, due to the following calculations
u.rewards + u.lastBalance * (block.timestamp - u.lastUpdate);
A malicious actor can call this function on behalf of a given user and reset the reward accrual by reseting the u.lastUpdate and therefor restarting a reward accrual proccess. This particular user now has less u.rewards than they should
Tools Used
Recommended Mitigation Steps
To mitigate add some kind of access control to require(user == msg.sender)