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

safeConcurTransfer can cause loss of funds #244

Closed
code423n4 opened this issue Feb 9, 2022 · 2 comments
Closed

safeConcurTransfer can cause loss of funds #244

code423n4 opened this issue Feb 9, 2022 · 2 comments
Labels
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 duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L201-L211

Vulnerability details

Impact

All calculations are rounded down, since a lack of tokens in the contracts cannot be rounding errors' fault. So the function is redundant.

On the other hand, if the contract is undersupplied with Concur tokens, this will cause depositors to be sent less tokens than needed (or none). This is especially unsafe because the tokens that were lacking are not resembled in accountings at all. Thus a depositor may invoke the safeConcurTransfer and not receive tokens they were supposed to.

Tools Used

Manual analysis

Recommended Mitigation Steps

Use usual safeTransfer instead of safeConcurTransfer.

@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 Feb 9, 2022
code423n4 added a commit that referenced this issue Feb 9, 2022
@r2moon r2moon added the duplicate This issue or pull request already exists label Feb 15, 2022
@r2moon
Copy link
Collaborator

r2moon commented Feb 15, 2022

duplicated with #262

@GalloDaSballo
Copy link
Collaborator

GalloDaSballo commented Apr 4, 2022

Duplicate of #262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants