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

Lack of event emission after critical initialize() functions #68

Open
code423n4 opened this issue Jun 23, 2021 · 3 comments
Open

Lack of event emission after critical initialize() functions #68

code423n4 opened this issue Jun 23, 2021 · 3 comments

Comments

@code423n4
Copy link
Contributor

Handle

0xRajeev

Vulnerability details

Impact

Most contracts use initialize() functions instead of constructor given the delegatecall proxy pattern. While most of them emit an event in the critical initialize() functions to record the init parameters for off-chain monitoring and transparency reasons, Ticket.sol nor its base class ControlledToken.sol emit such an event in their initialize() functions.

Impact: These contracts are initialized but their critical init parameters (name, symbol, decimals and controller address) are not logged for any off-chain monitoring.

Proof of Concept

See similar Medium-severity Finding M01 in OpenZeppelin’s audit of UMA protocol: https://blog.openzeppelin.com/uma-audit-phase-4/

https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/Ticket.sol#L24-L37

https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/ControlledToken.sol#L22-L36

Examples of event emission:
https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/PrizePool.sol#L239-L243

https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/YieldSourcePrizePool.sol#L47

Tools Used

Manual Analysis

Recommended Mitigation Steps

Emit an initialised event in Ticket.sol and ControlledToken.sol logging their init parameters.

@code423n4 code423n4 added 2 (Med Risk) bug Something isn't working labels Jun 23, 2021
code423n4 added a commit that referenced this issue Jun 23, 2021
@asselstine
Copy link
Collaborator

This is just event emission; it's severity is 0 (Non-critical).

@dmvt
Copy link
Collaborator

dmvt commented Aug 27, 2021

I'm going to split the difference here. Events are important for various reasons. In this case, due to the proxy pattern, the creation of the contract in the initialize function happen at the same time, making it trivial for a user to go back and look at the initialization parameters in the creation transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants