sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

KupiaSec - Lack of the access control in the `Voter::detachTokenFromGauge` function #638

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

KupiaSec

Medium

Lack of the access control in the Voter::detachTokenFromGauge function

Summary

Voter::detachTokenFromGauge function has the ability to detach certain tokens from the gauge and it is supposed to be called by privileged accounts, however the access control is missing and anyone can detach other user's token from the gauge.

Vulnerability Detail

Voter::detachTokenFromGauge function is as follows:


File: Voter.sol
444: function detachTokenFromGauge(uint tokenId, address account) external { // @audit should check the caller
445:         if (tokenId > 0) IVotingEscrow(_ve).detach(tokenId);
446:         emit Detach(account, msg.sender, tokenId);
447:     }

File: VotingEscrow.sol
1190: function detach(uint _tokenId) external {
1191:         require(msg.sender == voter);
1192:         attachments[_tokenId] = attachments[_tokenId] - 1;
1193:     }

As evident from the above code snippet, anyone can call this function for detaching tokens of other user's from the corresponding gauge as well as their tokens.

Hence it will affect several factor of the protocol, since the attachments check is used in several parts of the protocol like _transferFrom, withdraw, merge and split.

Impact

Anyone can detach token from the gauges and it will cause unintended behaviour in several factors of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Voter.sol#L444

Tool used

Manual Review

Recommendation

It is recommended to add a check in the Voter::detachTokenFromGauge as follows:


File: Voter.sol
444:    function detachTokenFromGauge(uint tokenId, address account) external { 
+            require(isGauge[msg.sender]);
+            require(isAlive[msg.sender]);   
445:         if (tokenId > 0) IVotingEscrow(_ve).detach(tokenId); 
446:         emit Detach(account, msg.sender, tokenId);
447:     }

Duplicate of #460