code-423n4 / 2023-08-pooltogether-findings

4 stars 3 forks source link

INCORRECT ACCESS CONTROL #175

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngAuction.sol#L170-L208 https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngAuction.sol#L288-L297 https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L131-L134 https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L171-L176 https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L181-L183 https://github.com/GenerationSoftware/pt-v5-vault-boost/blob/9d640051ab61a0fdbcc9500814b7f8242db9aec2/src/VaultBooster.sol#L201-L203

Vulnerability details

Impact

Access control plays an important role in segregation of privileges in smart contracts and other applications. If this is misconfigured or not properly validated on sensitive functions, it may lead to loss of funds, tokens and in some cases compromise of the smart contract.

The contracts RngAuction and VaultBooster are importing an access control library @openzeppelin/contracts/access/Ownable.sol but the functions (startRngRequest, getRngResults) and (getBoost, deposit, accrue, liquidatableBalanceOf) are missing the modifier onlyOwner.

Note: The functions in first set of brackets belong to the first contract and so on.

Proof of Concept

Read the following 2min read blog for understanding the concept. https://blog.solidityscan.com/access-control-vulnerabilities-in-smart-contracts-a31757f5d707

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to go through the contract and observe the functions that are lacking an access control modifier. If they contain sensitive administrative actions, it is advised to add a suitable modifier to the same

Assessed type

Access Control

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

raymondfam commented 1 year ago

Overinflated. Similarly QA was found in the bot.

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Invalid