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

[M-02] Lack of Minimum Delay Check in relyContract Function #739

Closed
c4-submissions opened this issue Sep 14, 2023 · 4 comments
Closed

[M-02] Lack of Minimum Delay Check in relyContract Function #739

c4-submissions opened this issue Sep 14, 2023 · 4 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-298 grade-c low quality report This report is of especially low quality QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/Cyfrin/2023-08-sparkn/blob/47c22b818818af4ea7388118dd83fa308ad67b83/src/ProxyFactory.sol#L205-L219

Vulnerability details

Vulnerable code:

        AuthLike(target).rely(user);
        emit RelyContract(target, user);
    }

Issue summary:
In the relyContract function of the smart contract, there is a missing minimum delay check when relying on another contract. This function allows the contract to rely on another contract, granting it certain privileges or permissions.

Vulnerability details:The vulnerability arises due to insufficient validation of the proxy parameter passed to the function.

Impact:
As per the documentation, relying on another contract should go through a time delay specified in the Root.delay variable, which is initially set to 48 hours. The absence of a minimum delay check means that this crucial security measure is not being enforced, potentially leading to unauthorized or unintended contract interactions.

Recommendations:
To address this issue and ensure the enforcement of the minimum delay requirement as per the documentation, consider implementing the following remediation steps:
1. Minimum Delay Check: Add a check in the relyContract function to ensure that the Root.delay time has passed before allowing the contract to rely on another contract. You can use a require statement to enforce this delay.
2. Time Management: Ensure that the contract's time management functions are correctly implemented, and the delay is accurately calculated based on the Root.delay variable.

Assessed type

Timing

@c4-submissions c4-submissions 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 Sep 14, 2023
c4-submissions added a commit that referenced this issue Sep 14, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as low quality report

@c4-pre-sort c4-pre-sort added the low quality report This report is of especially low quality label Sep 16, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #298

@c4-judge
Copy link

gzeon-c4 changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Sep 25, 2023
@c4-judge
Copy link

gzeon-c4 marked the issue as grade-c

@c4-judge c4-judge added grade-c unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-298 grade-c low quality report This report is of especially low quality QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants