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

Redundant allowance and balance checks #93

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

Redundant allowance and balance checks #93

code423n4 opened this issue Jun 16, 2021 · 3 comments

Comments

@code423n4
Copy link
Contributor

Handle

0xRajeev

Vulnerability details

Impact

In Market sponsor() the call to treasury.checkSponsorship() checks allowance and balance of user. This is redundant because the call to treasury.sponsor downstream checks allowance again and insufficient balance would cause any transfer to fail anyway.

Impact: Given the gas sensitivity of the code base, removing this redundant check could help conserve gas and prevent any DoS from breaking gas limits.

Proof of Concept

https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCMarket.sol#L810

https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCTreasury.sol#L386-L396

https://github.com/code-423n4/2021-06-realitycards/blob/86a816abb058cc0ed9b6f5c4a8ad146f22b8034c/contracts/RCTreasury.sol#L474-L478

Tools Used

Manual Analysis

Recommended Mitigation Steps

Remove redundant checks.

@code423n4 code423n4 added 1 (Low Risk) bug Something isn't working labels Jun 16, 2021
code423n4 added a commit that referenced this issue Jun 16, 2021
@Splidge
Copy link
Collaborator

Splidge commented Jun 18, 2021

conserve gas and prevent any DoS from breaking gas limits

Shouldn't this be Severity - G (Gas Optimisation)?

@Splidge
Copy link
Collaborator

Splidge commented Jun 21, 2021

removed redundant check here

@dmvt
Copy link
Collaborator

dmvt commented Jul 10, 2021

I agree... this is a gas optimization, not a risk issue

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

3 participants