Impact
A consensus-vulnerability in Geth could cause a chain split, where vulnerable versions refuse to accept the canonical chain.
Description
A flaw was repoted at 2020-08-11 by John Youngseok Yang (Software Platform Lab), where a particular sequence of transactions could cause a consensus failure.
-
Tx 1:
sender invokes caller.caller invokes 0xaa. 0xaa has 3 wei, does a self-destruct-to-selfcaller does a 1 wei -call to 0xaa, who thereby has 1 wei (the code in 0xaa still executed, since the tx is still ongoing, but doesn't redo the selfdestruct, it takes a different path if callvalue is non-zero)
-
Tx 2:
sender does a 5-wei call to 0xaa. No exec (since no code).
In geth, the result would be that 0xaa had 6 wei, whereas OE reported (correctly) 5 wei. Furthermore, in geth, if the second tx was not executed, the 0xaa would be destructed, resulting in 0 wei. Thus obviously wrong.
It was determined that the root cause was this commit from this PR. The semantics of createObject was subtly changd, into returning a non-nil object (with deleted=true) where it previously did not if the account had been destructed. This return value caused the new object to inherit the old balance:
func (s *StateDB) CreateAccount(addr common.Address) {
newObj, prev := s.createObject(addr)
if prev != nil {
newObj.setBalance(prev.data.Balance)
}
}It was determined that the minimal possible correct fix was
+++ b/core/state/statedb.go
@@ -589,7 +589,10 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)
s.journal.append(resetObjectChange{prev: prev, prevdestruct: prevdestruct})
}
s.setStateObject(newobj)
- return newobj, prev
+ if prev != nil && !prev.deleted {
+ return newobj, prev
+ }
+ return newobj, nilPatches
See above. The fix was included in Geth v1.9.20 "Paragade".
Credits
The bug was found by @johnyangk and reported via [email protected].
For more information
If you have any questions or comments about this advisory:
References
johnyangk Analyst
Impact
A consensus-vulnerability in Geth could cause a chain split, where vulnerable versions refuse to accept the canonical chain.
Description
A flaw was repoted at 2020-08-11 by John Youngseok Yang (Software Platform Lab), where a particular sequence of transactions could cause a consensus failure.
Tx 1:
senderinvokescaller.callerinvokes0xaa.0xaahas 3 wei, does a self-destruct-to-selfcallerdoes a1 wei-call to0xaa, who thereby has 1 wei (the code in0xaastill executed, since the tx is still ongoing, but doesn't redo the selfdestruct, it takes a different path if callvalue is non-zero)Tx 2:
senderdoes a 5-wei call to 0xaa. No exec (since no code).In geth, the result would be that
0xaahad6 wei, whereas OE reported (correctly)5wei. Furthermore, in geth, if the second tx was not executed, the0xaawould be destructed, resulting in0 wei. Thus obviously wrong.It was determined that the root cause was this commit from this PR. The semantics of
createObjectwas subtly changd, into returning a non-nil object (withdeleted=true) where it previously did not if the account had been destructed. This return value caused the new object to inherit the oldbalance:It was determined that the minimal possible correct fix was
Patches
See above. The fix was included in Geth
v1.9.20"Paragade".Credits
The bug was found by @johnyangk and reported via [email protected].
For more information
If you have any questions or comments about this advisory:
References