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

Timelocked functions doesn't emit proposal events #123

Open
code423n4 opened this issue Sep 21, 2021 · 2 comments
Open

Timelocked functions doesn't emit proposal events #123

code423n4 opened this issue Sep 21, 2021 · 2 comments
Labels
0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation bug Warden finding sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

nikitastupin

Vulnerability details

Usually timelock is used in order to give a users of a protocol time to react on protocol changes (e.g. to withdraw their funds). Thus timelock implementations have Proposal and Execution steps. The main way to monitor blockchain changes and react to them is to listen for emitted events. However, none of the timelocked functions (changePublisher, changeLicenseFee, publishNewIndex) emits an event on Proposal step (e.g. https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Basket.sol#L144-L147), they emit an event only on Execution step (e.g. https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Basket.sol#L143-L143).

Impact

Events aren't emitted at critical functions.

Proof of Concept

I'll write a PoC if needed.

Recommended Mitigation Steps

Add events after (1) https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Basket.sol#L145-L146, (2) https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Basket.sol#L163-L164, (3) https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Basket.sol#L189-L192 and https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Basket.sol#L182-L186.

@code423n4 code423n4 added 0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation bug Warden finding labels Sep 21, 2021
code423n4 added a commit that referenced this issue Sep 21, 2021
@frank-beard frank-beard added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Nov 6, 2021
@GalloDaSballo
Copy link
Collaborator

I completely disagree as you can listen for function invocations, it's offered on mainnet by theGraph and every major JS web3 package can handle them

@GalloDaSballo
Copy link
Collaborator

Am going to set the finding as valid because the sponsor confirmed, but I disagree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation bug Warden finding sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants