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

SingleNativeTokenExitV2 assumes first exchange holds the outputToken #176

Open
code423n4 opened this issue Dec 19, 2021 · 0 comments
Open
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 sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

kenzo

Vulnerability details

SingleNativeTokenExitV2 allows the user to exit and execute trades via multiple exchanges.
When finishing the trades and sending a single output token back to the user,
the contract takes that token from the last swap in the first exchange's trades.
There is nothing in the struct that signifies this will be the output token, and this also impairs the exit functionality.

Impact

Let's say a basket only holds token TOKE, and user would like to exit to DAI.
But there's no exchange with good liquidity for TOKE -> DAI.
So the user crafts a trade to exchange TOKE for WOKE in exchange A, and then exchange WOKE for DAI in exchange B, to finally receive back DAI. The contract will not let him do so, as the output token is taken to be the output token of the first exchange - WOKE in our example.

Proof of Concept

In exit, the output token is taken to be the last token exchanged in the first exchange: (Code ref)

address[] calldata path = _exitTokenStruct
            .trades[0]
            .swaps[_exitTokenStruct.trades[0].swaps.length - 1]
            .path;
        IERC20 outputToken = IERC20(path[path.length - 1]); //this could be not the target token

This manifests the issue I detailed above.

Recommended Mitigation Steps

Have the outputToken be a parameter supplied in ExitTokenStructV2.

@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 19, 2021
code423n4 added a commit that referenced this issue Dec 19, 2021
@loki-sama loki-sama added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jan 4, 2022
@loki-sama loki-sama added sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons and removed sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") labels Jan 17, 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 sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

2 participants