From ad24376b58c00c9d770ace5a3de42e416c4b7272 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Thu, 9 Jul 2020 13:37:33 +0200 Subject: [PATCH] Prevent contract funding from blocked addr --- x/wasm/internal/keeper/keeper.go | 7 ++ x/wasm/internal/keeper/keeper_test.go | 153 ++++++++++++++++++++++++-- x/wasm/internal/keeper/test_common.go | 17 +-- 3 files changed, 163 insertions(+), 14 deletions(-) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index a6916bf..d0d6908 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -179,6 +179,9 @@ func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.A // deposit initial contract funds if !deposit.IsZero() { + if k.bankKeeper.BlacklistedAddr(creator) { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "blocked address can not be used") + } sdkerr := k.bankKeeper.SendCoins(ctx, creator, contractAddress, deposit) if sdkerr != nil { return nil, sdkerr @@ -253,6 +256,10 @@ func (k Keeper) Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller // add more funds if !coins.IsZero() { + if k.bankKeeper.BlacklistedAddr(caller) { + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "blocked address can not be used") + } + sdkerr := k.bankKeeper.SendCoins(ctx, caller, contractAddress, coins) if sdkerr != nil { return nil, sdkerr diff --git a/x/wasm/internal/keeper/keeper_test.go b/x/wasm/internal/keeper/keeper_test.go index 570f152..2422bec 100644 --- a/x/wasm/internal/keeper/keeper_test.go +++ b/x/wasm/internal/keeper/keeper_test.go @@ -14,6 +14,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/supply" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" @@ -44,7 +45,7 @@ func TestCreate(t *testing.T) { wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") require.NoError(t, err) - contractID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "cosmwasm-opt:0.5.2", nil) + contractID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.NoError(t, err) require.Equal(t, uint64(1), contractID) // and verify content @@ -96,7 +97,7 @@ func TestCreateStoresInstantiatePermission(t *testing.T) { }) fundAccounts(ctx, accKeeper, myAddr, deposit) - codeID, err := keeper.Create(ctx, myAddr, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "cosmwasm-opt:0.5.2", nil) + codeID, err := keeper.Create(ctx, myAddr, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.NoError(t, err) codeInfo := keeper.GetCodeInfo(ctx, codeID) @@ -147,7 +148,7 @@ func TestCreateWithParamPermissions(t *testing.T) { params := types.DefaultParams() params.UploadAccess = spec.srcPermission keeper.setParams(ctx, params) - _, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "cosmwasm-opt:0.5.2", nil) + _, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.True(t, spec.expError.Is(err), err) if spec.expError != nil { return @@ -170,12 +171,12 @@ func TestCreateDuplicate(t *testing.T) { require.NoError(t, err) // create one copy - contractID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "cosmwasm-opt:0.5.2", nil) + contractID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.NoError(t, err) require.Equal(t, uint64(1), contractID) // create second copy - duplicateID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "cosmwasm-opt:0.5.2", nil) + duplicateID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.NoError(t, err) require.Equal(t, uint64(2), duplicateID) @@ -205,14 +206,14 @@ func TestCreateWithSimulation(t *testing.T) { require.NoError(t, err) // create this once in simulation mode - contractID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "confio/cosmwasm-opt:0.57.2", nil) + contractID, err := keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.NoError(t, err) require.Equal(t, uint64(1), contractID) // then try to create it in non-simulation mode (should not fail) ctx, keepers = CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) accKeeper, keeper = keepers.AccountKeeper, keepers.WasmKeeper - contractID, err = keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "confio/cosmwasm-opt:0.7.2", nil) + contractID, err = keeper.Create(ctx, creator, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "any/builder:tag", nil) require.NoError(t, err) require.Equal(t, uint64(1), contractID) @@ -316,6 +317,68 @@ func TestInstantiate(t *testing.T) { assert.Equal(t, info.Label, "demo contract 1") } +func TestInstantiateWithDeposit(t *testing.T) { + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + + var ( + bob = bytes.Repeat([]byte{1}, sdk.AddrLen) + fred = bytes.Repeat([]byte{2}, sdk.AddrLen) + + deposit = sdk.NewCoins(sdk.NewInt64Coin("denom", 100)) + initMsg = InitMsg{Verifier: fred, Beneficiary: bob} + ) + + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + + specs := map[string]struct { + srcActor sdk.AccAddress + expError bool + fundAddr bool + }{ + "address with funds": { + srcActor: bob, + fundAddr: true, + }, + "address without funds": { + srcActor: bob, + expError: true, + }, + "blocked address": { + srcActor: supply.NewModuleAddress(auth.FeeCollectorName), + fundAddr: true, + expError: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) + accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + + if spec.fundAddr { + fundAccounts(ctx, accKeeper, spec.srcActor, sdk.NewCoins(sdk.NewInt64Coin("denom", 200))) + } + contractID, err := keeper.Create(ctx, spec.srcActor, wasmCode, "https://github.com/CosmWasm/wasmd/blob/master/x/wasm/testdata/escrow.wasm", "", nil) + require.NoError(t, err) + + // when + addr, err := keeper.Instantiate(ctx, contractID, spec.srcActor, nil, initMsgBz, "my label", deposit) + // then + if spec.expError { + require.Error(t, err) + return + } + require.NoError(t, err) + contractAccount := accKeeper.GetAccount(ctx, addr) + assert.Equal(t, deposit, contractAccount.GetCoins()) + }) + } +} + func TestInstantiateWithPermissions(t *testing.T) { wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") require.NoError(t, err) @@ -480,6 +543,82 @@ func TestExecute(t *testing.T) { t.Logf("Duration: %v (%d gas)\n", diff, gasAfter-gasBefore) } +func TestExecuteWithDeposit(t *testing.T) { + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + + var ( + bob = bytes.Repeat([]byte{1}, sdk.AddrLen) + fred = bytes.Repeat([]byte{2}, sdk.AddrLen) + blockedAddr = supply.NewModuleAddress(auth.FeeCollectorName) + deposit = sdk.NewCoins(sdk.NewInt64Coin("denom", 100)) + ) + + specs := map[string]struct { + srcActor sdk.AccAddress + beneficiary sdk.AccAddress + expError bool + fundAddr bool + }{ + "actor with funds": { + srcActor: bob, + fundAddr: true, + beneficiary: fred, + }, + "actor without funds": { + srcActor: bob, + beneficiary: fred, + expError: true, + }, + "blocked address as actor": { + srcActor: blockedAddr, + fundAddr: true, + beneficiary: fred, + expError: true, + }, + "blocked address as beneficiary": { + srcActor: bob, + fundAddr: true, + beneficiary: blockedAddr, + expError: true, + }, + } + for msg, spec := range specs { + t.Run(msg, func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) + accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + + if spec.fundAddr { + fundAccounts(ctx, accKeeper, spec.srcActor, sdk.NewCoins(sdk.NewInt64Coin("denom", 200))) + } + codeID, err := keeper.Create(ctx, spec.srcActor, wasmCode, "https://example.com/escrow.wasm", "", nil) + require.NoError(t, err) + + initMsg := InitMsg{Verifier: spec.srcActor, Beneficiary: spec.beneficiary} + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + + contractAddr, err := keeper.Instantiate(ctx, codeID, spec.srcActor, nil, initMsgBz, "my label", nil) + require.NoError(t, err) + + // when + _, err = keeper.Execute(ctx, contractAddr, spec.srcActor, []byte(`{"release":{}}`), deposit) + + // then + if spec.expError { + require.Error(t, err) + return + } + require.NoError(t, err) + beneficiaryAccount := accKeeper.GetAccount(ctx, spec.beneficiary) + assert.Equal(t, deposit, beneficiaryAccount.GetCoins()) + }) + } +} + func TestExecuteWithNonExistingAddress(t *testing.T) { tempDir, err := ioutil.TempDir("", "wasm") require.NoError(t, err) diff --git a/x/wasm/internal/keeper/test_common.go b/x/wasm/internal/keeper/test_common.go index e49fb48..1657d8b 100644 --- a/x/wasm/internal/keeper/test_common.go +++ b/x/wasm/internal/keeper/test_common.go @@ -107,13 +107,6 @@ func CreateTestInput(t *testing.T, isCheckTx bool, tempDir string, supportedFeat auth.ProtoBaseAccount, // prototype ) - bankKeeper := bank.NewBaseKeeper( - accountKeeper, - paramsKeeper.Subspace(bank.DefaultParamspace), - nil, - ) - bankKeeper.SetSendEnabled(ctx, true) - // this is also used to initialize module accounts (so nil is meaningful here) maccPerms := map[string][]string{ auth.FeeCollectorName: nil, @@ -123,6 +116,16 @@ func CreateTestInput(t *testing.T, isCheckTx bool, tempDir string, supportedFeat staking.NotBondedPoolName: {supply.Burner, supply.Staking}, gov.ModuleName: {supply.Burner}, } + blockedAddr := make(map[string]bool, len(maccPerms)) + for acc := range maccPerms { + blockedAddr[supply.NewModuleAddress(acc).String()] = true + } + bankKeeper := bank.NewBaseKeeper( + accountKeeper, + paramsKeeper.Subspace(bank.DefaultParamspace), + blockedAddr, + ) + bankKeeper.SetSendEnabled(ctx, true) supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bankKeeper, maccPerms) stakingKeeper := staking.NewKeeper(cdc, keyStaking, supplyKeeper, paramsKeeper.Subspace(staking.DefaultParamspace))