code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L223-L246

Vulnerability details

Impact

The owner of the contract can send value by mistake when call setBaseURI and setFee, the ETH will stuck in the contract

Proof of Concept

function setBaseURI(string memory _baseURI) public payable onlyOwner { function setFee(uint256 _fee) public payable onlyOwner {

Tools Used

Review

Recommended Mitigation Steps

Remove the payable modifier of the setBaseURI and setFee functions

function setBaseURI(string memory _baseURI) public onlyOwner {
function setFee(uint256 _fee) public onlyOwner {

outdoteth commented 2 years ago

admin functions are marked as payable to save gas

HickupHH3 commented 2 years ago

Would've been great to mention this in the README in the optimizations section. I agree though, since they're privileged functions, chances of sending ETH together are low.

Downgrading to non-crit QA.