Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SETFEE() SHOULD BE TIMELOCKED #17

Closed
code423n4 opened this issue Jun 30, 2022 · 1 comment
Closed

SETFEE() SHOULD BE TIMELOCKED #17

code423n4 opened this issue Jun 30, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

Currently the implementation of PuttyV2.setFee() can be changed at any time although it emits the respective event and has a maximum value. Because the owner() of this contract can be any address (multisig, EOA, DAO multisig, etc.) there are no insights on how are going to be changed the fees and when.

Proof of Concept

Sticking up to the contract code itself, the fee can be changed freely within the given range. This means that for instance a malicious owner can be monitoring the mempool, frontrun other benign users and increase the fee up to 3% whenever he wants which will convey in a sort of fraud. If the attack is wanted to be performed in an even more intricate way, the owner can also backrun the transaction and take the fee back to what it was before this process.

Recommended Mitigation Steps

In order to prevent this, timelocking the setFee function will help provide trust and predictability on how the protocol will work.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jun 30, 2022
code423n4 added a commit that referenced this issue Jun 30, 2022
@outdoteth
Copy link
Collaborator

Duplicate: Admin can change fee at any time for existing orders: #422

@outdoteth outdoteth added the duplicate This issue or pull request already exists label Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants