🥔 add vulnerabilities

This commit is contained in:
bt3gl 2022-10-10 10:17:13 -07:00
parent 697082479f
commit c20a0c35fd
15 changed files with 1 additions and 1 deletions

View file

View file

@ -0,0 +1,35 @@
## 🌮 top public vulnerabilities reported to immunefi
<br>
* [Aurora inflation spend](aurora.md)
* [Wormhole Unitialized Proxy](wormhole.md)
* [Redacted Cartel Custom Approval Logic](redacted-cartel.md)
* [Optimism Infinite Money Duplication](optimism.md)
* [Nexus Mutual Bug](nexus.md)
* [Polygon Consensus Bypass](polygon.md)
* [Polygon Lack Of Balance](polygon2.md)
* [Polygon Double-Spend](polygon3.md)
* [APWine Incorrect Check of Delegations](apwine.md)
* [Notional Double Counting Free Collateral](notional.md)
* [Cronos Theft of Transactions Fees](cronos.md)
* [Enzyme Finance Price Oracle Manipulation](enzyme.md)
<br>
### Really good post-mortem
<br>
* [Sherlock Yield Strategy Bug Bounty Post-Mortem](https://mirror.xyz/0xE400820f3D60d77a3EC8018d44366ed0d334f93C/LOZF1YBcH1eBdxlC6HP223cAMeTpNgQ-Kc4EjQuxmGA)
<br>
### Other resources
<br>
* [crypto losses in Q3 2022](https://assets.ctfassets.net/t3wqy70tc3bv/6tpOiGqBr1VXs0AsnLwo1Y/33f0eb2e2a911d645bad874dad2ca73e/Immunefi_Crypto_Losses_in_Q3_2022_Report.pdf)
<br>

View file

@ -0,0 +1,15 @@
## APWine Incorrect Check of Delegations
<br>
* [Immunefi blog post](https://medium.com/immunefi/apwine-incorrect-check-of-delegations-bugfix-review-7e401a49c04f)
<br>
* The APWine protocol can be used to tokenize future yields.
* APWine operates by storing Interest Bearing Tokens (IBT) or any other yield-bearing asset in a smart contract for a specified period of time and issuing Future Yield Tokens (FYT) in exchange.
* Bug: in the PT tokens, one condition wasnt checked during the burn of those tokens which could lead to the theft of the yield from the protocol after the two periods, i.e. 6 months. (The condition is a `if` on an not address(0), which is called by a burn method).
* The division of a yield-bearing asset into Principal Tokens (PTs) and Future Yield Tokens is the essential functionality of APWine. A users deposits to the protocol are represented by the PTs. At the start of each period, the APWine generates FYT from PTs in a 1:1 ratio.

View file

@ -0,0 +1,55 @@
## Aurora inflation spend vulnerability
<br>
* [Immunefi blog post](https://medium.com/immunefi/aurora-infinite-spend-bugfix-review-6m-payout-e635d24273d)
<br>
### Bridges
* There arent any standardized ways of designing your own bridge. Were still in uncharted waters.
* Bridges also have additional “attack surface” as compared to “regular” DeFi projects. While a yield farm or a decentralized exchange might have a collection of smart contracts and a dapp web page, a bridge usually needs monitoring nodes, validator keys, and secure communication channels on top of those contracts and dapp.
* Most bridges implement "IOU" approcah: users send funds to the bridge protocol, where those funds are locked by the bridge smart contract. The bridge then issues the user an equivalent asset on the second network from the second bridge smart contract. The tokens on the destination chain are wrapped tokens. Bridges accumulate a lot of funds.
<br>
### Ethereums's `DELEGATECALL` vs. `CALL`
* If we use delegatecall() to call the native contract, the msg.value will be inherited from the original calling context, but the ETH is no longer passed to the native contract.
<img width="914" alt="Screen Shot 2022-06-08 at 8 29 36 PM" src="https://user-images.githubusercontent.com/1130416/172758401-b10c11fe-8fb4-42bf-b3be-3ffc4682ef9a.png">
<img width="929" alt="Screen Shot 2022-06-08 at 8 29 55 PM" src="https://user-images.githubusercontent.com/1130416/172758414-6a6d531d-dc6c-4507-b0cf-6cd63e4d0858.png">
<br>
### Aurora
* Aurora is an implementation of an EVM built on the NEAR network that supports all tools available in the Ethereum ecosystem.
* Aurora developed the Rainbow Bridge which allows users to transfers assets between Ethereum, NEAR, and Aurora.
* Two contracts in the Aurora Engine are interesting to us: `ExitToNear` and `ExitToEthereum`. They are special, built-in (precompiled) contracts that handle withdraw requests from the Aurora EVM.
<br>
### Exploit
Step by step:
1. Bridge Ether from Ethereum to Aurora using Rainbow Bridge (Aurora Bridge)
2. Deploy the malicious contract on Aurora that makes the delegatecall to the native contract ExitToNear i.e. 0xe9217bc70b7ed1f598ddd3199e80b093fa71124f
3. Call the exploit function of the malicious contract. Aurora is tricked at this point to send nETH to the caller on NEAR from the Aurora bridge contract. 4. The balance of attacker doesnt change on Aurora
5. Attacker then deposits back nETH to Aurora, doubling the attackers balance
6. Repeat from step 3.
<br>
### Vulnerability fix
* An exit error is returned if the address given does not match the input's address, which disables the ability to call the contract with `DELEGATECALL`.

View file

@ -0,0 +1,65 @@
## Cronos Theft of Transactions Fees
<br>
* [Immunefi blog post](https://medium.com/immunefi/cronos-theft-of-transactions-fees-bugfix-postmortem-b33f941b9570)
<br>
* Can we create a transaction that would be able to streal fees from the current block?
* By submitting a cosmos-signed `MsgEthereumTx` (with a valid Ethereum signature) without `ExtensionOptionsEthereu, Tx` and a high `GasLimit`, the attacker receives a gas refund even though the gas charge was not taken in the first place.
<br>
### Cronos Blockchain and Ethermint
* Built on the Cosmos SDK while being EVM compatible, Cronos is uniquely positioned to connect to the EVM chains and the Cosmos ecosystem, to enable seamless crypto assets and dApps interoperability in the multichain world.
* Ethermint is the underlying technology that Cronos is built on. Ethermint is a high-throughput, scalable PoS blockchain compatible with Ethereum. The Cosmos SDK, which operates on top of the Tendermint Core consensus engine, was used to create it.
* Since Ethermint is compatible with both EVM-based chains and Cosmos-specific blockchains, this also means the transactions can be bridged and propagated between the two chains. The client needs to parse and handle transactions routed for both the EVM and the Cosmos hub.
<br>
### EVM and Ethermint transactions
* Transactions are one of Ethereum's key functionalities, and they are the only thing that can modify or update the state of the blockchain. They are signed messages sent by the Ethereum network to every node via the "flood routing" protocol.
* New transactions looks as follow:
1. Nonce: an originating EOA-issued sequence number used to prevent message replay.
2. Gas price: how much Ether (in Wei) the originator is prepared to pay for each unit of gas.
3. Gas limit: the quantity of gas the originator is prepared to purchase for this transaction.
4. Recipient: the Ethereum address of the final destination (another EOA or contract address).
5. Value: the amount of Ether to transmit to the destination (in Wei).
6. Data: variable-length binary data payload.
7. v,r,s: the three components of the original EOA's ECDSA digital signature.
* The Ethermint's chain perform state transitions by using `MsgEthereumTx`. This message provides the relevant transaction data elements and wraps an Ethereum transaction as an SDK message.
* When transactions are consumed in Ethermint, they pass via a sequence of handlers. The `AnteHandler` is one of these handlers, and it's in change of conducting preliminary message execution business logic such as fee payment, signature verification, and so on. It only applies to transactions conducted through the Cosmos SDK. Because the EVM handles the same business logic, Ethereum routed transactions will be unaffected.
<br>
### Vulnerability Analysis
* Etheremint offers standard JSON-RPC endpoints to utilize whenever we wish to send an Ethereum transaction.
* When an Ethereum transaction is submitted through the compatibility JSON-RPC endpoint, it's reformatted as `MsgEthereumTx` and then has an `ExtensionOptionsEthereumTx` appended to it. When processed by a node, the `ExtensionOptionsEthreumTx` causes the transaction to be processed by the appropriate antehandlers, including the `EthGasConsume` handler. This handler deducts the gas price * gas limit from the originating account. After the transaction execution, the fee is refunded.
* This eliminates the need for a cosmos signature, while also ensuring that the gas limit indicated in the Ethereum message is subtracted first, as it would on Ethereum.
* However, there is no check to see if a `MsgEthereumTx` has an `ExtensionOptionsEthereumTx`. It's possible to package `MsgEthereumTX` without it, and submit it to the node. This executes only the standard set of antehandlers by omitting this option. As a result, we may skip a few antehandlers that are supposed to run before `MsgEthereumTx` is handled.
* One of the handlers that are called is `EthGasConsumeDecorator(evmkeeper)`. Its purpose is to ensure that the Ethereum transaction message has sufficient funds to cover intrinsic gas and that the sender has the funds to cover the gas cost. The quantity of gas consumed by a transaction before it is completed is referred to as intrinsic gas. This is different from the cosmos fee because it's meant to be refunded.
* But since the gas cost is not deducted, it's possible to specify an arbitrary gas limit and then get refunded gas that wasn't deducted, as the `EthGasConsumeDecorator` handler didn't run because the `ExtensionOptionsEthereumTx` was missing.
* This means that before the malicious transaction is being executed, the transaction fees in the current block might be seized and the attack could be repeated for every block.
<br>
### Vulnerabilty Fix
* Check if the `MsgEthereumTx` is contained inside the transaction.

View file

@ -0,0 +1,73 @@
## Enzyme Finance Price Oracle Manipulation
<br>
* [Immunefi blog post](https://medium.com/immunefi/enzyme-finance-price-oracle-manipulation-bug-fix-postmortem-4e1f3d4201b5)
<br>
* Using a flashloan from `IdleTokenGovernance.sol` affected the `totalSupply` fo the Idle tokens, which was used to calculate the price of the token.
* Price calculations were based on the `totalNav / totalSupply` of the tokens.
* It's worth noting the initial Idle Token integration was with v4, which did not have any flashloan logic. That was later added in v5, thus unintentionally introducing a bug into Enzyme's Finance protocol.
<br>
### Oracles and Flashloans
* A price oracle is a tool used to view the price information of a given asset. In the DeFi world, this means that whenever we want to know the price of a token, we use oracles.
* There are two types of oracles: Onchain and Offchain:
* Onchain oracles are smart contracts.
* Offchain oracles like Chainlink depend on the data being put into the smart contracts by a decentralized network. E.g. query Chainlink aggregator smart contract and get the latest price reported by the Chainlink nodes.
* Flashloans are a way to borrow a large amount of money from a lending protocol like Aave without collateralization, only for a certain fee paid at the end. The caveat is that it needs to be returned within one transaction.
* Flashloans use smart contracts. The main enforced rule is the borrower must pay back the loan before the transaction ends (otherwise, the contract reverses the transaction).
* The transaction to the flashloan contract can be divided into three parts (all in one transaction):
1. Receive the loan.
2. Perform actions with the loan.
3. Repay the loan.
<br>
#### Use cases
* Primary use case is for arbitrage. You can't purchase anything long-term with a flashloan, but you can make a profit with flashloan with an arbitrage opportunity.
* With substantial WETH/ETH available to us via flashloan, we can exchange them for any other asset we want through an AMM.
* If a token we're interested in only has a pool against DAI, we can swap flashloaned ETH for DAI and use some of the DAI we got for purchasing the token we are interested in, manipulating the price.
<br>
### Vulnerability analysis
* Enzyme Finance is an Ethereum-based protocol for decentralized on-chain asset management. It allows users and investors to create and invest funds.
* A fund owner configures the rules of their fund: fees and policies, the denomination asset by which share price and performance are measured, the time-lock between shares actions for a given user, etc.
* when we want to invest in a fund, we need to send the underlying asset of the fund to the vault. In return, users get shares that represent their part in the fund.
* Users can call `buyShares()` function on the `ComptrollerLib.sol`. We need to provide the amount of underlying assets of the fund with which we will be purchasing the share and the minimal amount of shares.
* How much we will need to pay for a share is calculated by the public function `calcGav()`. This function simply calculates the gross asset value (GAV) of the fund. This function consults `VaultInterpreter.sol` and `IDerivativePriceFeed.sol` for the canonical asset total value in a fund. The price feed we're interested in is `IdlePriceFeed.sol` for Idle Tokens.
* `IdlePriceFeed` gets the unit price of an Idle Token by calling `tokenPrice` function of the token. Return value of this function is something an attacker needs to manipulate to get better prices for a share when buying and selling for a profit.
* Looking at the `totNav` calculations, it seems like we could potentially manipulate one value here. The current balance of the Idle Token contract's backing assets, by using contract's internal flashloan functionality which would temporarily emptied the contract by flash loaning the max amount from the IdleToken.
<br>
### PoC
1. Fund malicious contract with WETH to be able to swap it later for USDC to pay for a flashloan.
2. Make a flashloan of IdleUSCDYield tokens. This will in fact, affect GAV calculations.
3. During a flashloan, call `buyShares`. As GAV calculations are affected, we are buying shares at a discount now.
4. Repay flashloan.
5. Call `redeemShares` to sell all the bought shares of Idle fund for a profit.
<br>
### Fix
* Delist IDLE tokens.

View file

@ -0,0 +1,24 @@
## Yearn.finance / Nexus Mutual Bug Bounty
<br>
* [Immunefi blog post](https://github.com/bt3gl-labs/Blockchain-Hacking-Toolkit/edit/main/Top-Immunefi-Vulnerabilities/nexus.md)
<br>
* The vulnerability consisted of an issue with the Single Sided Balancer (SSB) vaultsspecifically in the way the vault decided the number of BAL tokens to sell (LP tokens for Balancer).
* Before selling the yvUSDT, the attacker could take a flashloan of DAI or USDC to imbalance the pool.
* The attacker could then flash-borrow yvUSDT (this was the only vulnerable vault, due to the amount of liquidity on BentoBox) and withdraw everything.
The step-by-step guide to exploiting the now-patched bug is as follows:
```
1. Flash borrow yvUSDT and DAI from BentoBox
2. Buy USDT with DAI at Balancer to imbalance the pool
3. Withdraw from yvUSDT. Withdrawal will sell more Balancer LP tokens due to imbalanced pool
4. Buy DAI back with USDT to get a profit. (Pool is slightly more balanced because of previous step)
5. Deposit back to yvUSDT
6. Repay flashloan
```

View file

@ -0,0 +1,51 @@
## Notional Double Counting Free Collateral Bugfix Review
<br>
* [Immunefi report](https://medium.com/immunefi/notional-double-counting-free-collateral-bugfix-review-28b634903934)
<br>
* Bug could have allowed a user to drain almost all liquidity from markets on all currencies.
<br>
### Notional
* Notional is a fixed-term lending and borrowing platform on Ethereum.
* All operations done on the Notional V2 platform are done using their fCash token.
* fCash tokens are transferable tokens that indicate a claim on a positive or negative cash flow at a future date.
* The interest rate for lending and borrowing on Notional is set by fCash markets.
* Users deposit or receive cash in return for fCash when they lend or borrow at set rates.
<br>
### Bitmap Portfolio
* The portfolio is simply an array of assets, the number of assets is limited by governance-tunable parameters.
* There is a second "asset bitmap" portfolio which can be enabled on any account and is particularly handy for market makers.
* Bitmap portfolios, or asset bitmaps, must be activated as an account activity. Bitmap currencies can only be changed if there are no debts or credits in the bitmap asset.
* Before and after activating a bitmap portfolio, users do not need to make changes how they interact with the platform. Only fCas assets in the bitmap currency can be kept once a bitmap portfolio is activated.
* Only fCash assets in that bitmap currency can be kept once a bitmap portfolio is activated.
* `MAX_BITMAP_ASSETS` is the maximum number of fCash assets a bitmap portfolio can store.
<br>
### Vulnerability analysis
* The vulnerability lies inside the function `AccountContextHandler.enabledBitmapForAccount()` which is called from the account action contract.
* Once a bitmap currency is set, `enableBitmapForAccount` fails to clear the copy of the active bitmap currency in `accountContext.activeCurrencies` when the bitmap currency is changed. This is critical, as we can trigger double accounting of free collateral due to this,
1. Deposit a second currency (the one we want double accounted) by calling `depositUnderlyingToken()`. This will make a call to `setActiveCurrency` as flags argument and `isActive` being true, which will enable the currency in `accountContext.activeCurrencies` only if this currency is not active bitmap currency. One could force this condition to be true by calling `enableBitmapForAccount` with some dummy currency that we don't care about.
2. Then, one can make a second call to `enabledBitmapForAccount` after the deposit to change the bitmap currency to the one we deposited in the previous step. Due to the logic error, the free collateral calculations will be run twice on the asset we deposited: once for the bitmap and once for the `accountContext.activeCurrencies`.
3. This will trigger double account of free collateral of the activated currency. Free collateral represents the amount of collateral denominated in ETH that an account holds beyond what is needs to meet its minimum collateral requirements. If an account's free collateral figure is positive, the account is adequately collateralized. If the account's free collateral figure is negative, it's under-collateralized and eligible for liquidation. If calculation for free collateral are doubled (collateralization is high), this means the attacker could borrow more without actually holding enough collateral currency.

View file

@ -0,0 +1,35 @@
## Optimism Infinite Money Duplication
<br>
* [Immunefi blog post](https://medium.com/immunefi/optimism-infinite-money-duplication-bugfix-review-daa6597146a0)
* [Saurik blog post](https://www.saurik.com/optimism.html)
<br>
* The bug would have allowed an attacker to replicate money continuously on any chain using a vulnerability found in OVM 2.0.
* Executing means advancing the state of a blockchain by running transactions inside the virtual machine.
* The EVM has a stack-based architecture. We can control the stack by using opcodes. When smart contracts receive a message, their EVM bytecode is run, which allows them to update their state or even send further messages to other contacts.
* Proving is simply the act of convincing L1 contracts that the state produced by executing some transactions is correct.
* Optimisms approach to this is different from other L2 scaling solutions. Here, executing and proving are done together.
* If there were a dispute between Alice and Bob, the transaction in question would be re-executed (replayed) on the L1 chain. But this introduces some potential issues, as we cannot rely on certain opcodes to return the same value on the L1 chain and the L2 chain. Certain ones like BLOCKNUMBER, for example, wont produce the same value because they rely on blockchain metadata or information from the time of proving (instead of the time of execution).
* The solution is introducing a mechanism that would help retain the context of the disputed transaction on L2 when verifying it on L1. Optimism Virtual Machine (OVM 2.0) replaces all context-dependent opcodes with OVM counterparts, like ovmBLOCKNUMBER.
* The OVM 1.0 decided to store ETH as ERC20 tokens instead. This caused some issues for Optimism, as the network needed to support all things that were working on Ethereum but was broken due to this, like gas tokens. When OVM 2.0 launched, OVM 2.0 stopped support for this feature but still stores all of the balances for user accounts in the storage state of an ERC20 contract.
* Optimism modified Geth, one of the original three implementations of Ethereum protocol, to apply patches to StateDB to store the native balances in an ERC20 token storage state.
<br>
### The Vulnerability
[Code](https://github.com/MetisProtocol/mvm/blob/46d08bce46d1e0039a64522eb0bd9dfff0e0fc46/l2geth/core/state/statedb.go#L468)
* The `SELFDESTRUCT` opcode causes a contract to self-destruct. deleting its account object. This permits obsolete states to be removed from the active set of the blockchain.
* The `selfdestruct(address)` method, which was renamed from `suicide(address)`, destroys all bytecode from the calling contract address and transfers all Ether held to the target address. No functions (including the fallback) are called if the target address is also a contract.
* The most important thing to note here is that the contract (object) is destroyed at the end of a transaction, meaning we can still perform operations on that contract after calling the selfdestruct function, only if such operations are still in the same transaction.
* The problem is that the Optimism client sets the balance to zero directly on the stateObject instead of checking UsingOVM and redirecting balance modification to the OVM_ETH contract!
* Due to this bug, when a contract is selfdestructed, it gives the balance of the calling contract to the target AND still keeps the original balance. Were modifying a stateObject balance and not updating native balances in the ERC20 token storage state (OVM_ETH contract).
* An attacker could have used this bug to inflate the balance of the target contract by repeatedly selfdestructing a contract that holds Ether. After several iterations the attacker can then “cash out” the inflated Ether balance, thus creating the money out of thin air.

View file

@ -0,0 +1,68 @@
## Polygon Consensus Bypass Bugfix Review
<br>
* [Imunnefi blog post](https://medium.com/immunefi/polygon-consensus-bypass-bugfix-review-7076ce5047fe)
<br>
* What would happen if someone were able to bypass checks in the consensus requirement check to gain an advantage over the underline blockchain?
* This was a vulnerability in the proof of stake (PoS) system in Polygons smart contract on Ethereum, which would have allowed an attacker to decrease the total staking power, allowing a consensus (⅔ threshold) bypass that could potentially have allowed an attacker to drain all funds from the deposit manager, engage in unlimited withdrawals, DoS and more.
* For the attacker to have exploited this vulnerability, specific market conditions would have had to have been met. For example, a validator spot had to have been open, and the capital requirements were high (less capital means longer the attack takes). The amount to pay the miners directly to stay in the validator spot using flashbots was also high.
* A consensus mechanism helps prevent certain kinds of economic attacks. In theory, an attacker can compromise consensus by controlling 51% of the network. It was designed to make this “51% attack” unfeasible. A similar attack could be performed on the PoS consensus mechanism but instead of holding 51% of nodes, you need ⅔ +1 of the total staked amount.
* With an Ethereum Layer-1 PoW consensus mechanism, transactions are broadcasted to the mempools, with miners picking the transactions and processing them. However, due to high demand, this process is slower because of network congestion and nonviable gas prices. The goal of the scalability solution is to increase the transaction speed without sacrificing security or decentralization.
* Polygon PoS is a Layer-2 scalability solution that relies on a set of validators, who act like operators by staking the tokens into the system to secure the network.
* Those who are interested in securing the network but are not running a validator node can participate as delegators. The delegator role stakes their MATIC tokens to secure the Polygon network with existing validators without running the nodes themselves.
<br>
### Vulnerability analysis
* Polygons staking manager contract is the main contract for handling validator-related activities like checkpoint signature verification, reward distribution, slashing validators, and stake management.
* Validators are nodes that, among other things, send signed Polygon checkpoints to Ethereum that contain Merkle Tree hashes of the blocks in Polygon.
* For every validator participating in the contract via stakeFor() function, the contract asks the validator if they want to allow delegators to stake tokens into the validators acceptDelegation(bool). If the validator wants to accept delegators, then the contract creates a validatorShare delegator contract for the validators. The following screenshot represents the stakeFor() function and the described behavior.
* The StakingManager contract holds information about the validator state in the validatorState struct. The following are the two variables that are important to the vulnerability:
- validatorState.amount — The total number of tokens staked by the validators. In other words, it represents the total staking power.
- validatorState.stakerCount — Represents the total number of stakers in the contract.
* When a validator unstakes, the counter of the total staking power updates together with the delegated amount and the amount of the validator. This can be seen in the following line:
```
updateTimeline(-(int256(amount) + delegationAmount), -1, targetEpoch);
```
* The vulnerability arises when delegators migrate their delegations from one validator to another. The contract calls updateTimeline(-amount), which ends up subtracting the total validator power from the stakeManager contract, and once that validator unstakes, the counter of total staking power will be updated again by decreasing the validator amount + delegated amount again from the contract.
* So the exact issue lies in the updateValidatorState(validatorId, -int256(amount)) function on the stakeManager contract, where it modifies the timeline updateTimeline(amount, 0, 0); which reduces the total staking power validatorState.amount with the amount the delegator is migrating, without first checking if the current validator is currently staking or not in the contract.
<br>
### Steps to Reproduce
1. Create a new validator using the stakeFor function.
2. Call the buyVoucher function with a big delegated amount to buy the shares of the validators by staking tokens.
3. The attacker can now repeat the following steps until validatorState.amount (total staking power) is low enough to bypass the consensus majority check (⅔) requirement.
a. Catch an available validator slot via an on-chain auction process which happens at regular intervals.
b. Migrate staking tokens into that validator by calling a migrateDelegation function.
c. Unstake the validator. (validatorState.amount is decreased again)
d. Wait for a checkpoint (for this validator slot to open)
These steps will repeatedly decrease the total staking power by the same amount of delegated amount for each iteration. An attacker can repeat this until the total staking power is low enough to start accepting new checkpoints. He can bypass the required ⅔ consensus majority check. An attacker can lower the total staking power up to a low point that a sole validator can pass the majority check.

View file

@ -0,0 +1,43 @@
## Polygon Lack of Balance
<br>
* [Immunefi blog post](https://medium.com/immunefi/polygon-lack-of-balance-check-bugfix-postmortem-2-2m-bounty-64ec66c24c7d)
<br>
* This vulnerability consisted of a lack of balance/allowance check in the transfer function of Polygon's MRC20 contract and would have allowed an attacker to steal all MATIC.
<br>
### Vulnerability Analysis
* The MATIC token is like Ether, but for the Polygon network. The MATIC token is used in the Polygon ecosystem for several functions, including voting on PIPs, staking, and gas costs.
* The most interesting thing about MATIC token is its standard. It's the native gas-paying asset of the Polygon network, but it's also a contract deployed on Polygon. The contract is MRC20 contract. This standard is used mainly for the possibility of transferring MATIC gaslessly, which with Ether, is impossible. With Ether, you are making a transaction that a wallet needs to sign.
* Gasless MATIC transfers are facilitated by the `transferWithSig()` function. The user who owns the tokens signs a bundle of parameters, including the operator, amount, nonce, and expiration. This signature can be later passed to the MRC20 contract by the operator to perform a transfer on behalf of the token owner. This is gasless for the token owner because the operator pays for the gas.
* Smart contracts on Ethereum have access to the built-in ECDSA signature verification algorithm through `erecover`. This built-in function lets you verify the integrity of the signature over the hashed data and returns the signer's public key.
* `ecrecovery` is a wrapper function on top of the standard `erecover`, that lets you pass a packed signature without the need to separate V, R, and S.
* The bug in the token could have allowed an attacker to mint an arbitrary number of tokens from the MRC20 contract.
* The main issue is that `_transferFrom` will call the `_transfer` function directly without checking whether the `from` has enough balance. And we can call `transferWithSig()` without a valid signature, thanks to the lack of check to see if `erecovery` returns the zero address. The function takes the balances of `from` and `to` address and passes that to the `_transfer()`, which has the same issue (it doesn't check that the sender has enough balance).
<br>
### PoC
1. Create a byte string of length anything other than 65: `erecovery` returns the zero address if the packed signature does not have length 65. This means we don't need a valid signature to proceed.
2. `amount` passed to the function can be any amount, but we can use the full balance of the MRC20 contract.
3. `to` address will be an attacker address.
4. After `from` is recovered from the invalid signature, `_transferFrom()` is called.
5. As the balances are not checked from `from` and `to`, contracts makes a `_transfer()` call.
6. `_transfer()` only checks if the recipient isn't the MRC20 contract itself and transfers all the amount to the attacker from the MRC20 contract.
<br>
### Fix
* Remove `transferWithSig` function.

View file

@ -0,0 +1,34 @@
## Polygon Double-Spend Bugfix Review
<br>
* [Immunefi report](https://medium.com/immunefi/polygon-double-spend-bug-fix-postmortem-2m-bounty-5a1db09db7f1)
<br>
* Polygon introduced two bridges: Plasma (more secure) and PoS bridge.
* The main vulnerability lies in how Polygons WithdrawManager verifies the inclusion and uniqueness of the burn transaction in previous blocks.
<br>
<img width="904" alt="Screen Shot 2022-06-20 at 12 06 30 AM" src="https://user-images.githubusercontent.com/1130416/174544298-09f1fd2c-8413-497b-acfa-27a88a4980ec.png">
<br>
#### PoC
1. Deposit a large amount of ETH/tokens to Polygon through the Plasma Bridge.
2. After confirmation of the funds being available on the Polygon, start the Withdrawal process.
3. Wait for seven days for an exit to be valid.
4. Resubmit the exit payload but with a modified first byte of the branch mask.
5. The same valid transaction can be resubmitted up to 223 times with different values for the first byte of the HP-encoded path.
6. Profit.
<br>
### Fix
* The first byte of the encoded branch mask is supposed to always be 0x00.
* The fix is to check if the first byte of the encoded branch mask is 0x00 and not to disregard it as an incorrect mask.

View file

@ -0,0 +1,78 @@
## Redacted Cartel Custom Approval Logic Bugfix Review
<br>
* [Immunefi blog post](https://medium.com/immunefi/redacted-cartel-custom-approval-logic-bugfix-review-9b2d039ca2c5)
<br>
* The vulnerability was rated as critical because it would have allowed a malicious attacker to assign a users allowance to themselves, enabling the attacker to steal that users funds.
* The purpose of ERC20s `approve(spender, amount)` function is to allow any address to spend the tokens on behalf of the tokens owner.
* The vulnerability here consisted of a faulty implementation of standard ERC20 functions in REDACTEDs wxBTRFLY token, which is a wrapped version of the xBTRFLY.
### Vulnerability
The vulnerability can be seen at the `_approve()` call in the [contract](https://github.com/redacted-cartel/REDACTED-Smart-Contracts/blob/main/contracts/WXBTRFLY.sol#L826):
```
function transferFrom(address sender, address recipient, uint256 amount) public virtual override onlyAuthorisedOperators returns (bool) {
_transfer(sender, recipient, amount);
_approve(sender, msg.sender, allowance(sender, recipient ).sub(amount, "ERC20: transfer amount exceeds allowance"));
return true;
}
```
where `allowance(sender, recipient)` should be `allowance(sender, msg.sender)`.
<br>
#### Here is a clarification
| entity | address | description |
| ----------------- | ------------------- | -------------- |
| sender | `sender` | *from*; who holds the tokens before the transaction |
| recipient | `recipient` | *to*, who will receive the tokens after the transaction |
| spender | `msg.sender` | who is calling `transferFrom()`; the operator; who needs allowance approval |
<br>
[Here](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol) is how OpenZeppelin implements this function for `ERC20`:
```
function transferFrom(
address from,
address to,
uint256 amount
) public virtual override returns (bool) {
address spender = _msgSender();
_spendAllowance(from, spender, amount);
_transfer(from, to, amount);
return true;
}
```
where
```
function _spendAllowance(
address owner,
address spender,
uint256 amount
) internal virtual {
uint256 currentAllowance = allowance(owner, spender);
if (currentAllowance != type(uint256).max) {
require(currentAllowance >= amount, "ERC20: insufficient allowance");
unchecked {
_approve(owner, spender, currentAllowance - amount);
}
}
}
```

View file

@ -0,0 +1,125 @@
## Wormhole Uninitialized Proxy Bugfix Review
<br>
* [Immunefi post](https://medium.com/immunefi/wormhole-uninitialized-proxy-bugfix-review-90250c41a43a)
<br>
### Intro to Proxies
* A smart contract upgrade can be simply summarized as: a change in the code at a specific address while preserving the storage state of previous code and the relationship of that address to other contracts.
* A proxy contract and delegate calls can only swap the implementation, not the state of the contract.
* In Ethereum, there are three major types of contract calls: regular CALL, STATICCALL, and DELEGATECALL.
* When contract A makes a CALL to contract B by calling foo(), the function execution relies on contract Bs storage, and the msg.sender is set to contract A.
* However, when the same call is made using DELEGATECALL, the function foo() would be called on contract B but in the context of contract A. This means that the logic of contract B would be used, but any state changes made by the function foo() would affect the storage of contract A. And also, msg.sender would point to the EOA who made the call in the first place.
<img width="875" alt="Screen Shot 2022-06-08 at 10 22 23 PM" src="https://user-images.githubusercontent.com/1130416/172770375-6b99ee39-ebd8-4dc1-b8a1-e7c76f6161f4.png">
<br>
* A delegatecall makes it possible to create upgradeable contracts using a proxy pattern.
<img width="919" alt="Screen Shot 2022-06-08 at 10 23 34 PM" src="https://user-images.githubusercontent.com/1130416/172770543-32f49773-0025-45ae-acb1-5c8032432e96.png">
<br>
* Making an upgrade in this case is quite simple, as we only need to change the stored implementation contract address in order to change its smart contract logic.
<br>
### Transparent Proxy Pattern (TPP)
* when a proxy admin wants to call a proxy contract function transferOwnership() which shares a name with a function in the implementation contract, which one would be called?
* Transparent Proxy Pattern (TPP): calls by a user always execute using the implementation contracts logic. Calls by the proxy admin always execute using the proxy contracts logic.
* The transparent proxy needs additional logic in the proxy contract to manage all the upgradability functions, as well as the ability to identify whether the caller is the admin address. TPP is not as gas efficient as UUPS.
<br>
### Universal Upgradeable Proxy Standard (UUPS)
* With TPP, the upgrade logic is located in the proxy contract itself. But with UUPS, the upgrade logic is in the implementation contract.
* UUPS implementations have access to all the storage of the proxy; they can overwrite the storage slot of the proxy contract where the proxy stores the address of the implementation.
* We only check that the caller is the admin when an upgrade is requested. All authorization logic for upgradability is located within the implementation contract to guard against any unintended calls from happening.
<br>
### OpenZeppelin UUPS Uninitialized Proxies Vulnerability
* Wormhole vulnerability was detected by generalizing the pattern of the OpenZeppelin UUPS vulnerability.
* `initialize()` function calls `__Ownable_init, which sets the owner of the implementation contract to the first person to call it.
* Being an owner of the UUPS implementation contract means you can control the upgrade functions. The owner of the implementation can call upgradeToAndCall() directly on the implementation contract, instead of going through the proxy.
* The vulnerability lies in how `upgradeToAndCall()` works internally. Apart from changing the implementation address to a new one, it atomically executes any migration/initialization function using `DELEGATECALL` and the data passed along it. What would happen if somehow we managed to get the implementation contract to do an `upgradeToAndCall()` in its own context? This would cause the proxy contract to become useless, as it would forward all the calls to an empty address. Upgrading would no longer be possible.
<br>
### An Attack
1. The attacker calls initialize() on the implementation contract to become the owner. Remember the point above where initialize() makes the first person to call it the owner. Since nobody has called this function yet in the context of the implementation, the call works and makes the attacker the owner
Attacker deploys a malicious contract with a selfdestruct() function.
2. The attacker calls upgradeToAndCall() on the implementation contract as an owner, and points it to the malicious selfdestruct contract.
3. During the upgradeToAndCall() execution, DELEGATECALL is called from the implementation contract to the malicious selfdestruct contract using the context of the implementation contract (not the proxy).
4. SELFDESTRUCT is called, destroying the implementation contract.
5. The proxy contract is now rendered useless
<br>
### Wormhole Vulnerability
* Wormhole is also using a UUPS style proxy, where the upgrade logic resides in the implementation contract.
* The main difference is that the upgrade is guarded by Guardians that need to produce a multi-sig message stating the upgrade to the new implementation address is authorized.
* An implementation contract was uninitialized after a previous bugfix had reverted the original initialization. That means an attacker would be able to pass their own Guardian set and proceed with the upgrade as a Guardian they controlled.
* Once in control of the Guardian address, the attacker can use submitContractUpgrade() to force an upgrade attempt, causing a DELEGATECALL to an attacker-submitted address. If this address is a contract that executes a SELFDESTRUCT opcode, the implementation contract will be destroyed.
* The step-by-step guide to exploit is similar to the UUPS issue:
1. The attacker calls initialize() on the implementation contract to set the attacker controllable Guardian set.
2. Attacker deploys a malicious contract with a selfdestruct() function.
3. The attacker calls submitContractUpgrade() on the implementation contract and passes a signature signed by the malicious Guardian, which encodes the address of the malicious implementation contract for an upgrade.
4. During the submitContractUpgrade() execution, DELEGATECALL is called from the regular implementation contract to the malicious implementation contract.
5. SELFDESTRUCT is called, destroying the regular implementation contract.
6. The proxy contract is now rendered useless.
<br>
### Vulnerability fix
* The transaction called initialize() on the implementation contract and set the Guardians.