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

provideReinvest Can Be Manipulated Via A Sandwich Attacking To Extract More Malt Tokens From msg.sender #236

Closed
code423n4 opened this issue Dec 1, 2021 · 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

Handle

leastwood

Vulnerability details

Impact

getOptimalLiquidity is called from within provideReinvest to calculate how much Malt is required from the user. Subsequently, the contract transfers the correct amount of Malt tokens from msg.sender. As the caller of this function is unable to predetermine how much they should approve the contract as a spender, it is likely that users approve an unlimited amount. As a result, a well-funded attacker could alter the result of getOptimalLiquidity by impacting its underlying reserves, causing the contract to transfer more Malt tokens than the user expected. This could generate poor UX issues for users.

Proof of Concept

https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/RewardReinvestor.sol#L68-L71
https://github.com/code-423n4/2021-11-malt/blob/main/src/contracts/DexHandlers/UniswapHandler.sol#L111-L126

Tools Used

Manual code review

Recommended Mitigation Steps

provideReinvest should accept a maxMaltLiquidity parameter which should allow the user to limit the amount of tokens the spender (i.e. the RewardReinvestor contract) is able to receive.

@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 Dec 1, 2021
code423n4 added a commit that referenced this issue Dec 1, 2021
@0xScotch 0xScotch added the duplicate This issue or pull request already exists label Dec 10, 2021
@0xScotch
Copy link
Collaborator

#219

@GalloDaSballo
Copy link
Collaborator

Duplicate of #219

@GalloDaSballo GalloDaSballo marked this as a duplicate of #219 Jan 26, 2022
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

3 participants