search

code-423n4 / 2022-09-frax-findings

2 stars 1 forks source link

Toggle can be actioned by owner and time_lock contract at the same time #292

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L177 https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L184

Vulnerability details

Impact

Toggle can be actioned either by owner and timelock_address, this could be lead to confusion if both trigger the function at the same moment.

Tools Used

Visual Studio code review

Recommended Mitigation Steps

Emit Event helps, but it would be better to explicity pass a variable false /true or 0 / 1 and set submitPaused = 0 /1

FortisFortuna commented 2 years ago

We are well aware of the permission structure. The owner will most likely be a large multisig. We mentioned the Frax Multisig in the scope too. If moving funds, it is assumed someone in the multisig would catch an invalid or malicious address.

joestakey commented 2 years ago

This does not lead to an exploit, should be QA?

0xean commented 2 years ago

might be better to explicitly state a boolean state than a toggle. Definitely QA

0xean commented 2 years ago

closing, dupe of #294 - wardens QA