Simplified donation attack prevention

[sparrowDom]

We are planning to prevent big yield jumps by modifying code directly in VaultCore in this PR: https://github.com/OriginProtocol/origin-dollar/pull/2452 This allows for simplification in WOETH to completely rely on rebasingCreditsPerToken to deduce its own exchange rate.

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

[github-actions[bot]]

	Warnings
:warning:	:eyes: This PR needs at least 2 reviewers

Generated by :no_entry_sign: dangerJS against 87f08280b331e9e02c50a1f31b485fcf605b5605

[DanielVF]

🟠 We need to make sure that each implementation inherits from WOETH. WOSonic and WOETH currently do not.

[DanielVF]

🟠 Our initial target with this deploy is sonic, but we do not have a sonic deploy file, which also means this upgrade has not been fork tested on sonic.

[DanielVF]

🟥 There was a critical bug with rounding, allowing an attacker to steal from, and I think drain the contract.

totalAssets() rounds down. When it rounds down, it favors deposits. It's possible for an attacker to make a tiny mint to place the contract such that it has max down rounding error, then deposit a massive amount at an advantage. The attacker sizes this big amount, such that the tiny end then causes the ending rounding error to be minimal, and withdraws, collecting a profit.

I've added a draft of a fix for this. We basically just code our own simple convertToShares and convertToAssets that uses the current rebasingCreditsPerTokenHighres, and each round in the correct direction.

[DanielVF]

🟥 A mint at a zero totalSupply can mint at a privileged rate and thus make the contract insolvent. This does requires either a rebase to have happened between the initialize and the first mint, or the wrapped token to have gone back down to zero total supply and then a mint happen there.

Also fixed by the above changes.

In the old code, the deposit onto a zero supply mints at a 1:1 rate, ignoring the exchange rate.

[sparrowDom]

Sonic deploy and basic tests along with cotract extending WOETH done here: https://github.com/OriginProtocol/origin-dollar/pull/2453/commits/191f5225905a3ea99ba61688792c970c539108e6

[sparrowDom]

Requirements

The PR fixes a problem with our Wrapped OETH contract that was vulnerable to donation attacks. It also uses a unique way of using OETH's rebasingCreditsPerToken to trim down the math required to convert between assets and shares.

Easy Checks

Authentication

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

Ethereum

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

Cryptographic code

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

Gas problems

• [x] Contracts with for loops must have either: no for loops

Black magic

• [x] Does not contain selfdestruct
• [x] 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.
• [x] Address.isContract should be treated as if could return anything at any time, because that's reality.

Overflow

• [x] Code is solidity version >= 0.8.0

Proxy

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

Events

• [x] All state changing functions emit events

Medium Checks

Rounding and casts

• [x] Contract rounds in the protocols favor
  • convertToAssets will always round down. Meaning for a given amount of shares it will round the asset decimals down - like Math.floor(). Consequences:
    • maxWithdraw will report a rounded down amount of assets a user can withdraw - favouring the protocol
    • previewMint rounds up becase of convertToShares(convertToAssets(shares)) adding 1 wei when needed
    • previewRedeem will round down the amount of assets the user receives when redeeming shares.
    • 🟡 for all this to be safe the convertToShares(convertToAssets(shares)) must never return more than 1 wei difference in shares to the original shares passed to the convertToAssets. Basic fuzz testing confirmed this behavioue
  • convertToShares
    • previewDeposit rounds down when transforming assets to shares
    • is used in previewMint & previewWithdraw for the purposes of rounding up
  • most importantly non of the above functions include the contract's total supply, to it is immune to weird behaviour at specific totalSupply numbers.
• [x] Contract does not have bugs from loosing rounding precision
• [x] Code correctly multiplies before division
• [x] Contract does not have bugs from zero or near zero amounts
  • now that the contract doesn't use totalSupply in convertToShares & convertToAssets it should not misbehave when totalSupply is near zero
• [x] Safecast is aways used when casting. no safecast

Dependencies

• [X] Review any new contract dependencies thoroughly (e.g. OpenZeppelin imports) when new dependencies are added or version of dependencies changes.
  • OZ has done immense changes to 4626, but we've decided to not include them since precision math issues have been customely addressed by converting between assets and shares solely using OETH's rebasingCreditsPerToken
• [x] If OpenZeppelin ACL roles are use review & enumerate all of them.
• [x] Check OpenZeppelin security vulnerabilities and see if any apply to current PR considering the version of OpenZeppelin contract used.

External calls

• [x] Contract addresses passed in are validated
• [x] No unsafe external calls
  • we only call OETH and that is a trusted contract
• [x] Reentrancy guards on all state changing functions
  • no need as we only call the OETH trusted contarct
• [x] No malicious behaviors
• [x] Oracles
  • [x] reading rebasingCreditsPerToken from OETH and that one can not be manipulated in malicious manner
• [x] Do not call balanceOf for external contracts to determine what they will do when they use internal accounting

Tests

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

Deploy

• [x] Deployer permissions are removed after deploy

Strategy Specific

Remove this section if the code being reviewed is not a strategy.

Downstream

• [ ] We have monitoring on all backend protocol's governances
• [ ] We have monitoring on a pauses in all downstream systems

Thinking

Logic

Logic is really trimmed down to the bare necessities. Not using supply and totalAssets when converting between shares and assets makes logic very clean and robust.

• [X] Correct usage of global & local variables. -> they might differentiate only by an underscore that can be overlooked (e.g. address vs _address).

Deployment Considerations

The contract handles fresh WOETH deployment and existing one correctly by setting the right adjuster.

Internal State

There isn't much internal state to this contract. All it tracks is its totalSupply but importantly doesn't use it when doing calculation between assets and shares. There is an adjuster variable that gets set on deployment and that can't be manupulated. The important rebasingCreditsPerTokenHighres() is always being queried realtime from a trusted OETH contract.

Attack

A user somehow manipulating the conversion between shares and assets in such a way that it would cause rounding issues. Analysis of all possible paths hasn't revealed such possibility

Flavor

Code is very clean and probably can not reach further simplification.

[sparrowDom]

thanks @shahthepro all issues addressed:

• PR branched off of mainnet
• WrappedOUSD now extends WOETH
• added deploy file and test files for WOUSD