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

Lack of support for Fee-on-transfer / rebase token. #287

Closed
code423n4 opened this issue Nov 10, 2022 · 3 comments
Closed

Lack of support for Fee-on-transfer / rebase token. #287

code423n4 opened this issue Nov 10, 2022 · 3 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-367 satisfactory Finding meets requirement

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L223
https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L447

Vulnerability details

Impact

If the two parties, lender and borrower agree to use a fee-on-transfe token or a rebase token, the accounting will not work in line of credit contract.

Proof of Concept

Some tokens take a transfer fee (e.g. STA, PAXG), some do not currently charge a fee but may do so in the future (e.g. USDT, USDC).

For example, if the fee-on-transfer token charge 1% of fee on each transfer, the borrower and lender agree on a credit term,

the borrower call:

/// see ILineOfCredit.addCredit
function addCredit(
	uint128 drate,
	uint128 frate,
	uint256 amount,
	address token,
	address lender
)
	external
	payable
	override
	whileActive
	mutualConsent(lender, borrower)
	returns (bytes32)

the borrower transfer the 100 amount of token, but because 1% of the fee is charged, the smart contract received 99 amount of token.

but the internal accounting still use the original 100 amount of token

credits[id] = CreditLib.create(id, amount, lender, token, address(oracle));
ids.push(id); // add lender to end of repayment queue

The borrower want to borrow 100 amount token as they agreed on the term but the contract does not have enough balance, but the borrower still need to pay the interest as if there is 100 amount of token.

Some tokens may make arbitrary balance modifications outside of transfers (e.g. Ampleforth style rebasing tokens, Compound style airdrops of governance tokens, mintable / burnable tokens).

In fact, chainlink price oracle support Ampleforth token related oracle:
AMPL / ETH and AMPL / USD

https://docs.chain.link/docs/data-feeds/price-feeds/addresses/#Ethereum%20Mainnet

this is the contract for AMPL / USD oracle.
https://etherscan.io/address/0xe20CA8D7546932360e37E9D72c1a47334af57706#readContract

let us say the borrower and lender agrees on the term on a rebase token, since the contract does not track the rebasing balance, the rebased amount would not be accessible for both lender and borrower.

Tools Used

Manual Review.

Recommended Mitigation Steps

We recommend the project use before and after balance check to confirm how much amount of token the contract received.
We recommend the contract track the balance change for rebasing token.

@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 Nov 10, 2022
code423n4 added a commit that referenced this issue Nov 10, 2022
@c4-judge
Copy link
Contributor

dmvt marked the issue as duplicate of #26

@c4-judge c4-judge added the satisfactory Finding meets requirement label Dec 6, 2022
@c4-judge
Copy link
Contributor

c4-judge commented Dec 6, 2022

dmvt marked the issue as satisfactory

@C4-Staff
Copy link
Contributor

liveactionllama marked the issue as duplicate of #367

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-367 satisfactory Finding meets requirement
Projects
None yet
Development

No branches or pull requests

3 participants