code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

setBaseURI() and setFee() functions are payable but don't perform any logic on assets #383

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Any eth sent to these functions will be permanently lost

Proof of Concept

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

These functions shouldn't be payables

Recommended Mitigation Steps

Remove payable from the functions

GalloDaSballo commented 2 years ago

Admin functions are set as payable to save gas

outdoteth commented 2 years ago

Confirming what @GalloDaSballo said.

rotcivegaf commented 2 years ago

Duplicate of #259

HickupHH3 commented 2 years ago

User has no QA