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

Check made redundant by following check #48

Open
code423n4 opened this issue Aug 14, 2021 · 2 comments
Open

Check made redundant by following check #48

code423n4 opened this issue Aug 14, 2021 · 2 comments

Comments

@code423n4
Copy link
Contributor

Handle

0xRajeev

Vulnerability details

Impact

The check for array lengths is unnecessary in two places where the following check on txHash will anyway fail if the lengths don’t match with what was hashed earlier during schedule. Removing the length check can save a little gas.

Such a require() in cancel() can be removed because if there is a mismatch, the entry lookup in transactions[] will fail anyway and also, this will not be sceduled/executed.

Proof of Concept

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/TimeLock.sol#L70-L72

https://github.com/code-423n4/2021-08-yield/blob/4dc46470e616dd0cbd9db9b4742e36c4d809e02c/contracts/utils/TimeLock.sol#L81-L85

Tools Used

Manual Analysis

Recommended Mitigation Steps

Evaluate and remove these checks.

@alcueca
Copy link
Collaborator

alcueca commented Aug 16, 2021

Fix (TimeLock)

@alcueca
Copy link
Collaborator

alcueca commented Aug 16, 2021

Fix (EmergencyBrake)

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

2 participants