Context
Previously audited on:
Line of Credit + Spigot v1 FixesWhat's the problem
- Mainnet addresses
- Spigot that has the issue https://etherscan.io/address/0x6e3a81f41210d45a2bbbbad00f25fd96567b9af2
- Line that owns Spigot https://debtdao.finance/#/mainnet/lines/0x5bda5b7a953f71f03711f9c0bd2c10c1738f6ee4
- More tokens claimable by owner + operator from Spigot than Spigot actually holds
- Operator tokens is about right
- Owner tokens should be 2.7 from the 3 usdc rev but it's 4.2
Reproduction Case
<< Insert tests Craig writes >>
What we tried
- Seeing if UI/subgraph was wrong
- Checked that the claimOwnerTokens out equals amount escrowed in ClaimRevenue events
- Checked actual traded amount (dai + usdc)
- Checked unused tokens on line (dai + usdc)
Conclusion
- bug was introduced when we fixed a code4rena bug report by having Operator claim tokens in a separate tx instead of get sent directly in claimRevenue call. This means we have more tokens stored in the contract than we previously had to account for but we did not update this logic in claimRevenue. This causes more tokens to be claimed as revenue during push payments then actually were, falsely increased by the
state.operatorTokens
amount which we should be accounting for. - Allowed Operator and Owner to withdraw more tokens than should be allocated to them.
- Tests didnt pick it up because it only occurs on the second claimRevenue call, we mainly only test with one claimRevenue and if we do multiple we aren’t testing for operators balances, its usually for checking line’s unused tokens.
- FIX: Update https://github.com/debtdao/Line-of-Credit/blob/63faf71066f9e4ee9ac7311d4106ac550e79efbd/contracts/utils/SpigotLib.sol#L50 to include self.operatorTokens[token] in the claimed calculations for push payments