tornadocontrib/bsc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: add post-mortem (#23518)

Browse Source 
* docs: add post-mortem

* Update docs/postmortems/2021-08-22-split-postmortem.md

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>

* Update docs/postmortems/2021-08-22-split-postmortem.md

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>

* Update docs/postmortems/2021-08-22-split-postmortem.md

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>

* Update docs/postmortems/2021-08-22-split-postmortem.md

* Update 2021-08-22-split-postmortem.md

* Update docs/postmortems/2021-08-22-split-postmortem.md

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
This commit is contained in:
Martin Holst Swende 2021-09-07 09:22:40 +02:00 committed by GitHub
parent de2c44ab5c
commit 7957530225
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 266 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

266
docs/postmortems/2021-08-22-split-postmortem.md Normal file
View File

@ -0,0 +1,266 @@
# Minority split 2021-08-27 post mortem
This is a post-mortem concerning the minority split that occurred on Ethereum mainnet on block [13107518](https://etherscan.io/block/13107518), at which a minority chain split occurred.
## Timeline
- 2021-08-17: Guido Vranken submitted bounty report. Investigation started, root cause identified, patch variations discussed.
- 2021-08-18: Made public announcement over twitter about upcoming security release upcoming Tuesday. Downstream projects were also notified about the upcoming patch-release.
- 2021-08-24: Released [v1.10.8](https://github.com/ethereum/go-ethereum/releases/tag/v1.10.8) containing the fix on Tuesday morning (CET). Erigon released [v2021.08.04](https://github.com/ledgerwatch/erigon/releases/tag/v2021.08.04).
- 2021-08-27: At 12:50:07 UTC, issue exploited. Analysis started roughly 30m later,
## Bounty report
### 2021-08-17 RETURNDATA corruption via datacopy
On 2021-08-17, Guido Vranken submitted a report to bounty@ethereum.org. This co-incided with a geth-meetup in Berlin, so the geth team could fairly quickly analyse the issue.
He submitted a proof of concept which called the `dataCopy` precompile, where the input slice and output slice were overlapping but shifted. Doing a `copy` where the `src` and `dest` overlaps is not a problem in itself, however, the `returnData`slice was _also_ using the same memory as a backing-array.
#### Technical details
During CALL-variants, `geth` does not copy the input. This was changed at one point, to avoid a DoS attack reported by Hubert Ritzdorf, to avoid copying data a lot on repeated `CALL`s -- essentially combating a DoS via `malloc`. Further, the datacopy precompile also does not copy the data, but just returns the same slice. This is fine so far.
After the execution of `dataCopy`, we copy the `ret` into the designated memory area, and this is what causes a problem. Because we're copying a slice of memory over a slice of memory, and this operation modifies (shifts) the data in the source -- the `ret`. So this means we wind up with corrupted returndata.
```
1. Calling datacopy
memory: [0, 1, 2, 3, 4]
in (mem[0:4]) : [0,1,2,3]
out (mem[1:5]): [1,2,3,4]
2. dataCopy returns
returndata (==in, mem[0:4]): [0,1,2,3]
3. Copy in -> out
=> memory: [0,0,1,2,3]
=> returndata: [0,0,1,2]
```
#### Summary
A memory-corruption bug within the EVM can cause a consensus error, where vulnerable nodes obtain a different `stateRoot` when processing a maliciously crafted transaction. This, in turn, would lead to the chain being split: mainnet splitting in two forks.
#### Handling
On the evening of 17th, we discussed options how to handle it. We made a state test to reproduce the issue, and verified that neither `openethereum`, `nethermind` nor `besu` were affected by the same vulnerability, and started a full-sync with a patched version of `geth`.
It was decided that in this specific instance, it would be possible to make a public announcement and a patch release:
- The fix can be made pretty 'generically', e.g. always copying data on input to precompiles.
- The flaw is pretty difficult to find, given a generic fix in the call. The attacker needs to figure out that it concerns the precompiles, specifically the datcopy, and that it concerns the `RETURNDATA` buffer rather than the regular memory, and lastly the special circumstances to trigger it (overlapping but shifted input/output).
Since we had merged the removal of `ETH65`, if the entire network were to upgrade, then nodes which have not yet implemented `ETH66` would be cut off from the network. After further discussions, we decided to:
- Announce an upcoming security release on Tuesday (August 24th), via Twitter and official channels, plus reach out to downstream projects.
- Temporarily revert the `ETH65`-removal.
- Place the fix into the PR optimizing the jumpdest analysis [233381](https://github.com/ethereum/go-ethereum/pull/23381).
- After 4-8 weeks, release details about the vulnerability.
## Exploit
At block [13107518](https://etherscan.io/block/13107518), mined at (Aug-27-2021 12:50:07 PM +UTC), a minority chain split occurred. The discord user @AlexSSD7 notified the allcoredevs-channel on the Eth R&D discord, on Aug 27 13:09 UTC.
At 14:09 UTC, it was confirmed that the transaction `0x1cb6fb36633d270edefc04d048145b4298e67b8aa82a9e5ec4aa1435dd770ce4` had triggered the bug, leading to a minority-split of the chain. The term 'minority split' means that the majority of miners continued to mine on the correct chain.
At 14:17 UTC, @mhswende tweeted out about the issue [2].
The attack was sent from an account funded from Tornado cash.
It was also found that the same attack had been carried out on the BSC chain at roughly the same time -- at a block mined [12 minutes earlier](https://bscscan.com/tx/0xf667f820631f6adbd04a4c92274374034a3e41fa9057dc42cb4e787535136dce), at Aug-27-2021 12:38:30 PM +UTC.
The blocks on the 'bad' chain were investigated, and Tim Beiko reached out to those mining operators on the minority chain who could be identified via block extradata.
## Lessons learned
### Disclosure decision
The geth-team have an official policy regarding [vulnerability disclosure](https://geth.ethereum.org/docs/vulnerabilities/vulnerabilities).
> The primary goal for the Geth team is the health of the Ethereum network as a whole, and the decision whether or not to publish details about a serious vulnerability boils down to minimizing the risk and/or impact of discovery and exploitation.
In this case, it was decided that public pre-announce + patch would likely lead to sufficient update-window for a critical mass of nodes/miners to upgrade in time before it could be exploited. In hindsight, this was a dangerous decision, and it's unlikely that the same decision would be reached were a similar incident to happen again.
### Disclosure path
Several subprojects were informed about the upcoming security patch:
- Polygon/Matic
- MEV
- Avalanche
- Erigon
- BSC
- EWF
- Quorum
- ETC
- xDAI
However, some were 'lost', and only notified later
- Optimism
- Summa
- Harmony
Action point: create a low-volume geth-announce@ethereum.org email list where dependent projects/operators can receive public announcements.
- This has been done. If you wish to receive release- and security announcements, sign up [here](https://groups.google.com/a/ethereum.org/g/geth-announce/about)
### Fork monitoring
The fork monitor behaved 'ok' during the incident, but had to be restarted during the evening.
Action point: improve the resiliency of the forkmon, which is currently not performing great when many nodes are connected.
Action point: enable push-based alerts to be sent from the forkmon, to speed up the fork detection.
## Links
- [1] https://twitter.com/go_ethereum/status/1428051458763763721
- [2] https://twitter.com/mhswende/status/1431259601530458112
## Appendix
### Subprojects
The projects were sent variations of the following text:
```
We have identified a security issue with go-ethereum, and will issue a
new release (v1.10.8) on Tuesday next week.
At this point, we will not disclose details about the issue, but
recommend downstream/dependent projects to be ready to take actions to
upgrade to the latest go-ethereum codebase. More information about the
issue will be disclosed at a later date.
https://twitter.com/go_ethereum/status/1428051458763763721
```
### Patch
```diff
diff --git a/core/vm/instructions.go b/core/vm/instructions.go
index f7ef2f900e..6c8c6e6e6f 100644
--- a/core/vm/instructions.go
+++ b/core/vm/instructions.go
@@ -669,6 +669,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt
}
stack.push(&temp)
if err == nil || err == ErrExecutionReverted {
+ ret = common.CopyBytes(ret)
scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret)
}
scope.Contract.Gas += returnGas
@@ -703,6 +704,7 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([
}
stack.push(&temp)
if err == nil || err == ErrExecutionReverted {
+ ret = common.CopyBytes(ret)
scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret)
}
scope.Contract.Gas += returnGas
@@ -730,6 +732,7 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext
}
stack.push(&temp)
if err == nil || err == ErrExecutionReverted {
+ ret = common.CopyBytes(ret)
scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret)
}
scope.Contract.Gas += returnGas
@@ -757,6 +760,7 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext)
}
stack.push(&temp)
if err == nil || err == ErrExecutionReverted {
+ ret = common.CopyBytes(ret)
scope.Memory.Set(retOffset.Uint64(), retSize.Uint64(), ret)
}
scope.Contract.Gas += returnGas
diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go
index 9cf0c4e2c1..9fb83799c9 100644
--- a/core/vm/interpreter.go
+++ b/core/vm/interpreter.go
@@ -262,7 +262,7 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
// if the operation clears the return data (e.g. it has returning data)
// set the last return to the result of the operation.
if operation.returns {
- in.returnData = common.CopyBytes(res)
+ in.returnData = res
}
switch {
```
### Statetest to test for the issue
```json
{
"trigger-issue": {
"env": {
"currentCoinbase": "b94f5374fce5edbc8e2a8697c15331677e6ebf0b",
"currentDifficulty": "0x20000",
"currentGasLimit": "0x26e1f476fe1e22",
"currentNumber": "0x1",
"currentTimestamp": "0x3e8",
"previousHash": "0x0000000000000000000000000000000000000000000000000000000000000000"
},
"pre": {
"0x00000000000000000000000000000000000000bb": {
"code": "0x6001600053600260015360036002536004600353600560045360066005536006600260066000600060047f7ef0367e633852132a0ebbf70eb714015dd44bc82e1e55a96ef1389c999c1bcaf13d600060003e596000208055",
"storage": {},
"balance": "0x5",
"nonce": "0x0"
},
"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
"code": "0x",
"storage": {},
"balance": "0xffffffff",
"nonce": "0x0"
}
},
"transaction": {
"gasPrice": "0x1",
"nonce": "0x0",
"to": "0x00000000000000000000000000000000000000bb",
"data": [
"0x"
],
"gasLimit": [
"0x7a1200"
],
"value": [
"0x01"
],
"secretKey": "0x45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"
},
"out": "0x",
"post": {
"Berlin": [
{
"hash": "2a38a040bab1e1fa499253d98b2fd363e5756ecc52db47dd59af7116c068368c",
"logs": "1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"indexes": {
"data": 0,
"gas": 0,
"value": 0
}
}
]
}
}
}
```