CVE-2022-35936: Exposure of Resource to Wrong Sphere (CWE-668)
value: Ethermint is an Ethereum library. In Ethermint running versions before `v0.17.2`, the contract `selfdestruct` invocation permanently removes the corresponding bytecode from the internal database storage. However, due to a bug in the `DeleteAccount`function, all contracts that used the identical bytecode (i.e shared the same `CodeHash`) will also stop working once one contract invokes `selfdestruct`, even though the other contracts did not invoke the `selfdestruct` OPCODE. This vulnerability has been patched in Ethermint version v0.18.0. The patch has state machine-breaking changes for applications using Ethermint, so a coordinated upgrade procedure is required. A workaround is available. If a contract is subject to DoS due to this issue, the user can redeploy the same contract, i.e. with identical bytecode, so that the original contract's code is recovered. The new contract deployment restores the `bytecode hash -> bytecode` entry in the internal state.
CWE description: The product exposes a resource to the wrong control sphere, providing unintended actors with inappropriate access to the resource.
CVSS v3 base score: 5.3
Published: 2022-08-05T13:15Z
Repository: https://github.com/evmos/ethermint
Language: Go
Files changed: x/evm/keeper/statedb.go, x/evm/keeper/statedb_test.go
Commit date: 2022-08-04 21:47:01 +0200
Commit message: Merge pull request from GHSA-f92v-grc2-w2fg

* remove delete code in DeleteAccount

* update unit test

* refactor test

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Diff: diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go
index a70d142..32efb19 100644
--- a/x/evm/keeper/statedb.go
+++ b/x/evm/keeper/statedb.go
@@ -1,9 +1,8 @@
 package keeper
 
 import (
-	"bytes"
 	"fmt"
 	"math/big"
 
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -184,26 +183,20 @@ func (k *Keeper) DeleteAccount(ctx sdk.Context, addr common.Address) error {
 	if acct == nil {
 		return nil
 	}
 
 	// NOTE: only Ethereum accounts (contracts) can be selfdestructed
-	ethAcct, ok := acct.(ethermint.EthAccountI)
+	_, ok := acct.(ethermint.EthAccountI)
 	if !ok {
 		return sdkerrors.Wrapf(types.ErrInvalidAccount, "type %T, address %s", acct, addr)
 	}
 
 	// clear balance
 	if err := k.SetBalance(ctx, addr, new(big.Int)); err != nil {
 		return err
 	}
 
-	// remove code
-	codeHashBz := ethAcct.GetCodeHash().Bytes()
-	if !bytes.Equal(codeHashBz, types.EmptyCodeHash) {
-		k.SetCode(ctx, codeHashBz, nil)
-	}
-
 	// clear storage
 	k.ForEachStorage(ctx, addr, func(key, _ common.Hash) bool {
 		k.SetState(ctx, addr, key, nil)
 		return true
 	})
diff --git a/x/evm/keeper/statedb_test.go b/x/evm/keeper/statedb_test.go
index 52b29e2..e2c82a6 100644
--- a/x/evm/keeper/statedb_test.go
+++ b/x/evm/keeper/statedb_test.go
@@ -13,10 +13,11 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/evmos/ethermint/crypto/ethsecp256k1"
 	"github.com/evmos/ethermint/tests"
 	"github.com/evmos/ethermint/x/evm/statedb"
 	"github.com/evmos/ethermint/x/evm/types"
 )
 
@@ -418,16 +419,30 @@ func (suite *KeeperTestSuite) TestSuicide() {
 	}
 
 	suite.Require().NoError(db.Commit())
 	db = suite.StateDB()
 
+	// Generate 2nd address
+	privkey, _ := ethsecp256k1.GenerateKey()
+	key, err := privkey.ToECDSA()
+	suite.Require().NoError(err)
+	addr2 := crypto.PubkeyToAddress(key.PublicKey)
+
+	// Add code and state to account 2
+	db.SetCode(addr2, code)
+	suite.Require().Equal(code, db.GetCode(addr2))
+	for i := 0; i < 5; i++ {
+		db.SetState(addr2, common.BytesToHash([]byte(fmt.Sprintf("key%d", i))), common.BytesToHash([]byte(fmt.Sprintf("value%d", i))))
+	}
+
 	// Call Suicide
 	suite.Require().Equal(true, db.Suicide(suite.address))
 
 	// Check suicided is marked
 	suite.Require().Equal(true, db.HasSuicided(suite.address))
 
+	// Commit state
 	suite.Require().NoError(db.Commit())
 	db = suite.StateDB()
 
 	// Check code is deleted
 	suite.Require().Nil(db.GetCode(suite.address))
@@ -439,10 +454,14 @@ func (suite *KeeperTestSuite) TestSuicide() {
 	})
 	suite.Require().Equal(0, len(storage))
 
 	// Check account is deleted
 	suite.Require().Equal(common.Hash{}, db.GetCodeHash(suite.address))
+
+	// Check code is still present in addr2 and suicided is false
+	suite.Require().NotNil(db.GetCode(addr2))
+	suite.Require().Equal(false, db.HasSuicided(addr2))
 }
 
 func (suite *KeeperTestSuite) TestExist() {
 	testCases := []struct {
 		name     string

Vulnerable code: 	"bytes"
	ethAcct, ok := acct.(ethermint.EthAccountI)
	// remove code
	codeHashBz := ethAcct.GetCodeHash().Bytes()
	if !bytes.Equal(codeHashBz, types.EmptyCodeHash) {
		k.SetCode(ctx, codeHashBz, nil)
	}

Fixed code: 	_, ok := acct.(ethermint.EthAccountI)
	"github.com/evmos/ethermint/crypto/ethsecp256k1"
	// Generate 2nd address
	privkey, _ := ethsecp256k1.GenerateKey()
	key, err := privkey.ToECDSA()
	suite.Require().NoError(err)
	addr2 := crypto.PubkeyToAddress(key.PublicKey)

	// Add code and state to account 2
	db.SetCode(addr2, code)
	suite.Require().Equal(code, db.GetCode(addr2))
	for i := 0; i < 5; i++ {
		db.SetState(addr2, common.BytesToHash([]byte(fmt.Sprintf("key%d", i))), common.BytesToHash([]byte(fmt.Sprintf("value%d", i))))
	}

	// Commit state

	// Check code is still present in addr2 and suicided is false
	suite.Require().NotNil(db.GetCode(addr2))
	suite.Require().Equal(false, db.HasSuicided(addr2))