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.

Sign up for GitHub

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

Jump to bottom

Reentrancy hook on bounty #256

Closed
code423n4 opened this issue Sep 22, 2021 · 3 comments
Closed

Reentrancy hook on bounty #256

code423n4 opened this issue Sep 22, 2021 · 3 comments
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Warden finding duplicate Another warden found this issue sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

goatbug

Vulnerability details

Impact

Anyone can add any bounty and pass in any smart contract instead of an erc20 token.
When calling settleAuction, this triggers a tranfer function to be called from the smart contract that can be used to re-enter the auction or basket code.

Currently this cannot do anything malicious, but after the code is heavily refactored and improved based on the audit, it is possible this reentrancy hook could maliciously be used.

The contract should rather have a whitelist of token addresses that could be used for the bounty to avoid reentrancy.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Warden finding labels Sep 22, 2021
code423n4 added a commit that referenced this issue Sep 22, 2021
@code423n4
goatbug issue #256
b5b44bc
@frank-beard frank-beard added duplicate Another warden found this issue sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") labels Oct 19, 2021
@frank-beard
Copy link
Collaborator

frank-beard commented Oct 19, 2021 •
edited
Loading

dduplicate of #31

@GalloDaSballo
Copy link
Collaborator

Duplicate of #136

@GalloDaSballo GalloDaSballo marked this as a duplicate of #136 Dec 19, 2021
@GalloDaSballo
Copy link
Collaborator

After re-review this is a duplicate of #270
Mentions re-entrancy without a poc

@GalloDaSballo GalloDaSballo added 1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Dec 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1 (Low Risk) Assets are not at risk. State handling, function incorrect as to spec, issues with comments bug Warden finding duplicate Another warden found this issue sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@GalloDaSballo @code423n4 @frank-beard