Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

attens - When swapping 18-decimal token to 8-decimal token , user could buy decimal-18-token with 0 amount of decimal-8-token #245

Open
Attens1423 opened this issue Jul 17, 2023 · 3 comments
Labels
Medium A valid Medium severity issue Will Fix The sponsor confirmed this issue will be fixed

Comments

@Attens1423
Copy link

Attens1423 commented Jul 17, 2023

medium

When swapping decimal-18-token to decimal-8-token , user could buy decimal-18-token with 0 amount of decimal-8-token

Summary

Due to calculation errors, users can purchase a certain quantity of 18-decimal tokens (such as DAI) using 8-decimal tokens with a quantity of 0 (e.g., WBTC on ETH).

Vulnerability Detail

Here is the poc:

uint256 payFromToken = d3Proxy.buyTokens(
            address(d3MM),
            user1,
            address(token1),
            address(token2),
            10000000,
            0,
            abi.encode(swapData),
            block.timestamp + 1000
        );
assertEq(payFromToken, 0);

Impact

It may cause unexpected loss

Tool Used

Manual Review

Recommendation

In buyToken() of D3Trading.sol, add this rule:

if(payFromAmount == 0) { // value too small
            payFromAmount = 1;
        }
@Attens1423 Attens1423 added Medium A valid Medium severity issue Will Fix The sponsor confirmed this issue will be fixed labels Jul 17, 2023
@Attens1423 Attens1423 changed the title attens - Whenswap decimal-18-token to decimal-8-token , user could buy decimal-18-token with 0 amount of decimal-8-token attens - When swapping 18-decimal token to 8-decimal token , user could buy decimal-18-token with 0 amount of decimal-8-token Jul 17, 2023
@Attens1423
Copy link
Author

@hrishibhat
Copy link

Please note: This issue is not part of the contest submissions and is not eligible for contest rewards.

@IAm0x52
Copy link
Collaborator

IAm0x52 commented Sep 8, 2023

Fix looks good. Previously if swapping a lower precision token, the precision loss could be abused to by small amounts of higher DP tokens, though the amount gained would almost always be too small for any abuse profitably. 0 amount from token swaps are now always rounded up to 1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants