WOETH - Fixed yield rate each day

[DanielVF]

Code Change Checklist

To be completed before internal review begins:

• The contract code is complete
• Executable deployment file
• Fork tests that test after the deployment file runs
• Unit tests *if needed
• The owner has done a full checklist review of the code + tests

Internal review:

• Two approvals by internal reviewers

Deploy checklist

Two reviewers complete the following checklist:

- [ ] All deployed contracts are listed in the deploy PR's description
- [ ] Deployed contract's verified code (and all dependencies) match the code in master
- [ ] Contract constructors have correct arguments
- [ ] The transactions that interacted with the newly deployed contract match the deploy script.
- [ ] Governance proposal matches the deploy script
- [ ] Smoke tests pass after fork test execution of the governance proposal

[github-actions]

	Warnings
⚠️	👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against 8f8363c

[codecov]

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes missing coverage. Please review.

Please upload report for BASE (sparrowDom/woeth_hack_proof@79af39f). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
contracts/contracts/token/WOETH.sol	92.50%	3 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                       @@
##             sparrowDom/woeth_hack_proof    #2421   +/-   ##
==============================================================
  Coverage                               ?   14.22%           
==============================================================
  Files                                  ?      100           
  Lines                                  ?     4900           
  Branches                               ?     1294           
==============================================================
  Hits                                   ?      697           
  Misses                                 ?     4199           
  Partials                               ?        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rappie]

OETH -> WOETH

[DanielVF]

Code Review - WOETH

Requirements

We want to ensure that no donations can instantly affect the exchange rate used by lending platforms. This is because an instantly changing exchange rate on a redeemable asset can be fatal to a lending platform that uses it for borrows.

The approach we chose here controls the exchange rate by slowly streaming out additional assets over time.

Asset yield streaming

By controlling the extra asset amounts, rather than the rate, we avoids the easy critical bugs around fixed exchange rate changes in the face of changing asset amounts.

🟣 However a fixed asset per second yield does mean that the exchange rate can change over the yield period, as the total supply changes. If we assume that the standard attack requires a 2x donation, if an attacker perfectly time this around the end of a yield period, once they redeemed their half of funds, the rate of change of the exchange rate would double. However this still spreads their donation out over 23 hours, which still cancels out the attack.

No flash loans

A core defense of this contract is that donations can only affect subsequent blocks. It is impossible to use a flash loan and see even the slightest change in exchange rate inside the same block.

🟣 However, a whale attacker can reduce the cost of their donation by coming back into woeth after the attack has landed, and collecting a portion of the streaming yield. While this reduces the cost of the attack, it does not reduce the massively increased size of the donation needed.

Sample attacks:

For each attack, we will assume that an attacker needs a 50% price instant increase for the attack to land. It might be less, but it’s what we have seen in past attacks. It must be one block, because otherwise liquidations would rekt the attacker.

Assuming 5 million wOETH supply. 10 million wOETH supply.

Required donation sizes:

Old WOETH: 5 million
No direct donation WOETH: 10 million
This contract, on 12 seconds per block: 34,500 million.

A flash loan cannot be used to help with this.

Having a scheduled future yield does allow others to join the contract only when this contracts yield is known to be higher than other opportunities. This does however require them to already hold OETH, or the conversion fees or redeem time would probably preclude profitability.

Easy Checks

Authentication

• Never use tx.origin
• Every external/public function is supposed to be externally accessible
• Every external/public function has the correct authentication
• Every use of msg.sender is correct

Ethereum

• Contract does not send or receive Ethereum.
• Contract has no payable methods.
• Contract does not have a vulnerablity from being sent self destruct ETH

Cryptographic code

• This contract code does not roll it's own crypto.
• No signature checks without reverting on a 0x00 result.
• No signed data could be used in a replay attack, on our contract or others.

Gas problems

No for loops

Black magic

No magic

Overflow

• Code is solidity version >= 0.8.0
• All for loops use uint256

Proxy

• No storage variable initialized at definition when contract used as a proxy implementation.

Events

• All state changing functions emit events

Medium Checks

Rounding and casts

• Contract rounds in the protocols favor
• Contract does not have bugs from loosing rounding precision
  • 🟣 We send funds to a dead address
• Code correctly multiplies before division
• Contract does not have bugs from zero or near zero amounts
  • 🟣 We send funds to a dead address
• Safecast is aways used when casting
  • 🟣 Used when needed

Dependencies

• Review any new contract dependencies thoroughly (e.g. OpenZeppelin imports) when new dependencies are added or version of dependencies changes.
• If OpenZeppelin ACL roles are use review & enumerate all of them.
• Check OpenZeppelin security vulnerabilities and see if any apply to current PR considering the version of OpenZeppelin contract used.

External calls

• Contract addresses passed in by users are validated
• No unsafe external calls
• Reentrancy guards on all state changing functions
  • 🟣 Not needed, since only interacting with one trusted contract.

Tests

• Each publicly callable method has a test
• Each logical branch has a test
  • 🟠 Should ensure we have a test for a loss of balance case.
• Each require() has a test
• Edge conditions are tested

Deploy

• Deployer permissions are removed after deploy

Logic

Looks good.

Deployment Considerations

No

Internal State

• yieldAssets should be smaller than trackedAssets
• totalAssets() should never be larger than trackedAssets
• totalAssets() should never be smaller than trackedAssets - yieldAssets

Attack

This code controls 30%-50% of our our assets. It’s widely used. A critical failure could allow these assets to be stolen.

This code is also used by lending platforms or other DeFi systems, and wrong values from the exchange rate could result in losses there beyond just our token.

The two primary attacks against this kind of contract are rounding errors and donation attacks. We think we’ve blocked both of these.

Flavor

This code errs on the side of simplicity, using if statements to clearly define edge case behavior, as well as named intermediate variables to keep things easy to understand.

• Functions follow validate, read, compute, write, call, validate

[sparrowDom]

@DanielVF minor code change: 72a1292

[sparrowDom]

Requirements

This PR prepares the WOETH contract to be safe to use on lending platforms by preventing the possibility of rapid inflation / donation attack to either the WOETH contract or the OETH Vault.

Easy Checks

Authentication

• Never use tx.origin
• Every external/public function is supposed to be externally accessible
• Every external/public function has the correct authentication

Ethereum

• Contract does not send or receive Ethereum.
• Contract has no payable methods.
• Contract is not vulnerable to being sent self destruct ETH

Cryptographic code

• This contract code does not roll it's own crypto.
• No signature checks without reverting on a 0x00 result.
• No signed data could be used in a replay attack, on our contract or others.

Gas problems

• No loops

Black magic

• Does not contain selfdestruct
• Does not use delegatecall outside of proxying. If an implementation contract were to call delegatecall under attacker control, it could call selfdestruct the implementation contract, leading to calls through the proxy silently succeeding, even though they were failing.
• Address.isContract should be treated as if could return anything at any time, because that's reality.

Overflow

• Code is solidity version >= 0.8.0
• no for loops

Proxy

• No storage variable initialized at definition when contract used as a proxy implementation.

Events

• All state changing functions emit events

Medium Checks

Rounding and casts

• Contract rounds in the protocols favor
• 🟡 Contract does not have bugs from loosing rounding precision
  • contract can loose precision when rounding especially when the value of shares is inflated. For each order of magnitude difference between underlying assets and shares the error grows. Roughly 10 wei to the pow of orders of magnitude difference.
• Code correctly multiplies before division
• 🟡 Contract does not have bugs from zero or near zero amounts
  • it does. For this reason on a fresh deploy we need to mint a sufficient amount of WOETH and send it to a dead address. A small amount of shares (e.g. 1e9) is sufficient.
• 🟡 Safecast is aways used when casting
  • it is on all but 1 place - and it is not a safecast there intentionally

Dependencies

• Review any new contract dependencies thoroughly (e.g. OpenZeppelin imports) when new dependencies are added or version of dependencies changes.
• ~~If OpenZeppelin ACL roles are use review & enumerate all of them.~~no roles
• Check OpenZeppelin security vulnerabilities and see if any apply to current PR considering the version of OpenZeppelin contract used.

External calls

• Contract addresses passed in are validated
• No unsafe external calls
• 🟡 Reentrancy guards on all state changing functions
  • not needed, since the 1 contract called is OToken trusted contract
• No malicious behaviors
• Oracles, one of:
  • No oracles
  • Oracles can't be bent
  • If oracle can be bent, it won't hurt us.

Tests

• Each publicly callable method has a test
• Each logical branch has a test
• Each require() has a test
• Edge conditions are tested

Deploy

• Deployer permissions are removed after deploy

Thinking

Logic

Looks good.

Deployment Considerations

No

Internal State

• trackedAssets should be smaller or equal to totalAssets
• trackedAssets - yieldAssets should be a positive number
• yieldEnd should never be more than 23 hours into the future from the current time

Attack

Attacks on wrapper contracts like ours will usually try either a donation attack or a rounding attack when contract balances are small. This PR addresses the possibility of a donation attack to either the WOETH contract or the OUSD Vault.

Flavor

The contract needs to represent the funds entering / exiting as part of wrapping and un-wrapping the token and the other funds which are result of yield & donations. We current have trackedAssets representing the assets entering, exiting & yield assets. Separate yieldAssets representing only the yield assets. It would be more organic to have a storage variable for assets only entering / exiting without including yield assets.

Though it is easier to understand there was a lot of casting necessary making the code uglier.

Yield limit 🟣

There is a way to game the 5% daily yield limit the contract sets as a limitation. The steps are:

• wait for the contract to not be emitting yield
• take a flash loan and borrow great amounts of WETH
• mint OETH and deposit it to WOETH ( this will kick off yield start)
• redeem all the WOETH
• redeem all the OETH for WETH (and pay 0.1% redemption fee)

The exchange rate between OETH and WOETH still takes 23 hours to drip. That 5% daily (23 hours rather) yield limitation can be increased to any amount by paying the 0.1% in redemption fees. Considering the ~40m in WOETH TVL and a flash loan of 500m, the 5% daily rebase limit could be increased by 12.5x to a 62.5% limit.

Since no yield is dripped in the first (flash loan) transaction, this makes the attack much more risky.

[sparrowDom]

@DanielVF there is a Yield Limit issue described at the bottom of my internal review comment above. The almost same issue has been identified by OZ team a couple of minutes after my post.

Here is the proposal for the mitigation for the attack: #2437

[DanielVF]

"You're being let go... Your department's being downsized. You're part of an outplacement. We're going in a different direction. We're not picking up your option..."

We've decided on not using this approach, and instead controlling yield inside the vault.