-
Notifications
You must be signed in to change notification settings - Fork 3
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
Approve to 0 first to be compatible with USDT-like tokens #473
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate-1782
sufficient quality report
This report is of sufficient quality
unsatisfactory
does not satisfy C4 submission criteria; not eligible for awards
Comments
code423n4
added
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
labels
Aug 30, 2023
bytes032 marked the issue as duplicate of #928 |
bytes032 marked the issue as not a duplicate |
bytes032 marked the issue as duplicate of #1455 |
bytes032 marked the issue as duplicate of #1782 |
bytes032 marked the issue as not a duplicate |
bytes032 marked the issue as duplicate of #1782 |
bytes032 marked the issue as sufficient quality report |
c4-pre-sort
added
the
sufficient quality report
This report is of sufficient quality
label
Sep 11, 2023
GalloDaSballo marked the issue as unsatisfactory: |
c4-judge
added
the
unsatisfactory
does not satisfy C4 submission criteria; not eligible for awards
label
Oct 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate-1782
sufficient quality report
This report is of sufficient quality
unsatisfactory
does not satisfy C4 submission criteria; not eligible for awards
Lines of code
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L200
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L204
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L328
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L169
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L173
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L302
Vulnerability details
Impact
If router spends less tokens than desired, there will some dust tokens left, and there will left allowance on ERC20 with this dust amount. Some tokens like USDT revert when set non-zero allowance from non-zero allowance.
In case when this situation occurs, and router doesn't spend all allowance, further interaction will be broken because of revert on
approve()
Proof of Concept
I mentioned all the occurrences of vulnerable setting approval in "Links to affected code".
The context is following: UniV3LiquidityAmo.sol and UniV2LiquidityAmo.sol interact with Uniswap router, on every operation they set new allowance, but as described this can revert. As a result, contracts cannot perform further operations because of reverting approve
Tools Used
Manual Review
Recommended Mitigation Steps
Firstly make force approve to zero
Assessed type
ERC20
The text was updated successfully, but these errors were encountered: