<!-- canonical: https://0xsimao.com/findings/teller-finance-burn-shares-withdraw-share -->

# `burnSharesToWithdrawEarnings` burns before math, causing the share value to increase

Crit/High · Sherlock · Lending · 23rd April 2024

Finding H-2 of the Teller Finance competition.

- Protocol: https://audits.sherlock.xyz/contests/295
- Codebase: https://github.com/0xsimao/2024-04-teller-finance/tree/defe55469a2576735af67483acf31d623e13592d
- Source: https://github.com/sherlock-audit/2024-04-teller-finance-judging/issues/19

---

## Summary
The [burnSharesToWithdrawEarnings](https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L403-L408) function burns shares before calculating the share price, resulting in an increase in share value and causing users to be overpaid.

## Vulnerability Detail
When `burnSharesToWithdrawEarnings` burns shares and subsequently calculates the principal per share, the share amount is reduced but the principal remains the same, leading to a decrease in the `sharesExchangeRateInverse`, which increases the principal paid.

Principal is calculated using [_valueOfUnderlying](https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L324), where the formula is:
```solidity
uint256 principalTokenValueToWithdraw = (amount * 1e36) / sharesExchangeRateInverse();
```
The problem arises in [sharesExchangeRateInverse](https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L277) which uses the reduced share count:
```solidity
return  1e72 / sharesExchangeRate();
```
Where [sharesExchangeRate](https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L262) is defined as:
```solidity
rate_ = (poolTotalEstimatedValue * 1e36) / poolSharesToken.totalSupply();
```
Example:
- 1k shares and 10k principal.
1. Alice withdraws 20% of the shares.
2. 200 shares are burned, reducing the supply to 800.
3. Calculation for principal:

`rate_ = (10_000e18 * 1e36) / 800e18 = 1.25e37`
`sharesExchangeRateInverse = 1e72 / 1.25e37 = 8e34`
`principalTokenValueToWithdraw = 200e18 * 1e36 / 8e34 = 2500e18`

4. Alice withdraws 25% of the pool's principal, despite owning only 20% of the shares.

## Impact
This flaw results in incorrect calculations, allowing some users to withdraw more than their fair share, potentially leading to insolvency.

## Code Snippet
```solidity
poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);
uint256 principalTokenValueToWithdraw = _valueOfUnderlying(
    _amountPoolSharesTokens,
    sharesExchangeRateInverse()
);
```

## Tool used
Manual Review

## Recommendation
To ensure accurate withdrawal amounts, calculate `principalTokenValueToWithdraw` before burning the shares.

---

Related findings:

- [Liquidating maturies with unassigned earnings will not take into account floating assets increase leading to loss of funds](https://0xsimao.com/findings/exactly-protocol-update-staking-contract-ii-liquidating-maturies-unassigned-floating): Exactly Protocol Update - Staking Contract
- [`totalEarnings` is incorrect when withdrawing after ending which will withdraw too many funds leaving the `Vault` insolvent](https://0xsimao.com/findings/saffron-lido-vaults-ending-withdraw-leaving-insolvent): Saffron Lido Vaults
- [`borrowing::withdraw()` at a loss will increase downside protected and misscalculate option fees](https://0xsimao.com/findings/autonomint-withdraw-downside-misscalculate-fees): Autonomint
- [Borrower deposit, withdraw, deposit will reinit `omniChainData.cdsPoolValue`, getting profit stuck for cds depositors](https://0xsimao.com/findings/autonomint-deposit-withdraw-reinit-stuck): Autonomint