Platform: Code4rena
Start Date: 09/12/2022
Pot Size: $90,500 USDC
Total HM: 35
Participants: 84
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 12
Id: 192
League: ETH
Rank: 51/84
Findings: 2
Award: $134.51
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xsomeone
Also found by: 0xhacksmithh, 8olidity, Critical, Ruhum, SamGMK, Secureverse, Tointer, __141345__, aviggiano, rotcivegaf
133.3608 USDC - $133.36
https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/StableVault.sol#L78-L83 https://github.com/code-423n4/2022-12-tigris/blob/b2ebb8ea1def4927a747e7a185174892506540ab/contracts/StableVault.sol#L65-L72
The current StableVault design states that its purpose is to allow for many tokens of the same kind (such as USDC/DAI/USDT) to be used to trade on Tigris without splitting up the liquidity into multiple individual pools. Each one of these underlying tokens is redeemable 1:1 by their tigUSD vault token counterpart.
The problem arises when the vault contains more than one type of token and one of those depegs, which may render the vault insolvent. For example, if the vault supports USDC/DAI/USDT and USDT goes to zero (because of regulatory issues, market conditions, etc.), users might want to withdraw all the other assets, which makes the other valuable assets "tainted" together with a zero-valued asset on the same pool.
Manual analysis
Consider modifying the StableVault design mechanism to a battle-tested one similar to Curve's StableSwap, which takes into account market behavior on the liquidity pool.
#0 - c4-judge
2022-12-20T16:12:41Z
GalloDaSballo marked the issue as duplicate of #462
#1 - c4-judge
2022-12-20T16:13:19Z
GalloDaSballo changed the severity to 3 (High Risk)
#2 - c4-judge
2023-01-22T17:36:57Z
GalloDaSballo marked the issue as satisfactory
🌟 Selected for report: 0xA5DF
Also found by: 0xA5DF, 0xNazgul, 0xSmartContract, 0xbepresent, 0xdeadbeef0x, 8olidity, Englave, Faith, HE1M, JohnnyTime, Madalad, Mukund, Ruhum, SmartSek, __141345__, aviggiano, carlitox477, cccz, chaduke, francoHacker, gz627, gzeon, hansfriese, hihen, imare, jadezti, kwhuo68, ladboy233, orion, peanuts, philogy, rbserver, wait, yjrwkk
1.1472 USDC - $1.15
The Tigris protocol does not have enough boundary conditions that prevent a malicious owner to steal users' funds.
By using some onlyOwner actions, it is possible, for example, to call Trading.setMaxWinPercent to any arbitrary value, including one greater than 100%, which would make it possible to open a position and close it with an arbitrarily high payout, allowing it to steal all the vault's funds.
Even if we trust the protocol owner, we believe this is an unnecessary and serious centralization risk. A malicious or compromised owner address can take advantage of this, and steal all the funds from the protocol.
Trading.setMaxWinPercent, setting the max win percent as an extremely large number, enough to empty the whole vault.Manual analysis
Impose boundary checks for all owner-controlled actions. For example, a maximum value for maxWinPercent. Review other functions with onlyOwner modifier and validate if the scope of these actions can be limited in order to reduce the damage of a potential incident.
#0 - TriHaz
2022-12-26T08:22:06Z
We are aware of the centralization risks, owner of all contracts will be the multi-sig treasury for now, until we have a fully decentralized DAO.
#1 - c4-sponsor
2022-12-26T08:22:12Z
TriHaz marked the issue as sponsor acknowledged
#2 - c4-judge
2023-01-15T15:52:12Z
GalloDaSballo marked the issue as duplicate of #377
#3 - GalloDaSballo
2023-01-15T15:52:22Z
Admin privilege risk
#4 - c4-judge
2023-01-22T17:34:41Z
GalloDaSballo marked the issue as satisfactory