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

No event for Alchemist.sol#setPegMinimum #24

Open
code423n4 opened this issue Nov 17, 2021 · 2 comments
Open

No event for Alchemist.sol#setPegMinimum #24

code423n4 opened this issue Nov 17, 2021 · 2 comments
Labels
0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") wont fix

Comments

@code423n4
Copy link
Contributor

Handle

0x0x0x

Vulnerability details

Impact

The change of pegMinimum is crucial for the funcionality of the contract. Users should be informed about the changes. Furthermore, when pegMinimum is set to be maximum of uin256, functions such as mint, liquidate and repay cannot be used. Therefore, the change of pegMinimum should be emitted to create a safe environment for users.

Tools Used

Manual analysis

Recommended Mitigation Steps

Emit the changes. Furthermore, it would be better if for such a change users get notified beforehand with a mechanism such as Timelock.

@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 Nov 17, 2021
code423n4 added a commit that referenced this issue Nov 17, 2021
@Xuefeng-Zhu Xuefeng-Zhu added sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) labels Nov 23, 2021
@Xuefeng-Zhu
Copy link
Collaborator

should be low risk

@0xleastwood
Copy link
Collaborator

Marking as non-critical as event handling has no direct security risk.

@0xleastwood 0xleastwood added 0 (Non-critical) Code style, clarity, syntax, versioning, off-chain monitoring (events etc), exclude gas optimisation and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Dec 21, 2021
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 Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") wont fix
Projects
None yet
Development

No branches or pull requests

3 participants