Merge pull request #850 from CosmWasm/840-enforce-default-code-permissions

Enforce default code permissions

Author: ethanfrey

x/wasm/keeper/keeper.go

@@ -159,6 +159,15 @@ func (k Keeper) create(ctx sdk.Context, creator sdk.AccAddress, wasmCode []byte,
 	if !authZ.CanCreateCode(k.getUploadAccessConfig(ctx), creator) {
 		return 0, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not create code")
 	}
+	// figure out proper instantiate access
+	defaultAccessConfig := k.getInstantiateAccessConfig(ctx).With(creator)
+	if instantiateAccess == nil {
+		instantiateAccess = &defaultAccessConfig
+	} else if !instantiateAccess.IsSubset(defaultAccessConfig) {
+		// we enforce this must be subset of default upload access
+		return 0, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "instantiate access must be subset of default upload access")
+	}
+
 	wasmCode, err = ioutils.Uncompress(wasmCode, uint64(types.MaxWasmSize))
 	if err != nil {
 		return 0, sdkerrors.Wrap(types.ErrCreateFailed, err.Error())
@@ -175,10 +184,6 @@ func (k Keeper) create(ctx sdk.Context, creator sdk.AccAddress, wasmCode []byte,
 	}
 	codeID = k.autoIncrementID(ctx, types.KeyLastCodeID)
 	k.Logger(ctx).Debug("storing new contract", "features", report.RequiredFeatures, "code_id", codeID)
-	if instantiateAccess == nil {
-		defaultAccessConfig := k.getInstantiateAccessConfig(ctx).With(creator)
-		instantiateAccess = &defaultAccessConfig
-	}
 	codeInfo := types.NewCodeInfo(checksum, creator, *instantiateAccess)
 	k.storeCodeInfo(ctx, codeID, codeInfo)
 

x/wasm/keeper/keeper_test.go

@@ -183,6 +183,84 @@ func TestCreateWithParamPermissions(t *testing.T) {
 	}
 }
 
+// ensure that the user cannot set the code instantiate permission to something more permissive
+// than the default
+func TestEnforceValidPermissionsOnCreate(t *testing.T) {
+	ctx, keepers := CreateTestInput(t, false, SupportedFeatures)
+	keeper := keepers.WasmKeeper
+	contractKeeper := keepers.ContractKeeper
+
+	deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000))
+	creator := keepers.Faucet.NewFundedAccount(ctx, deposit...)
+	other := keepers.Faucet.NewFundedAccount(ctx, deposit...)
+
+	onlyCreator := types.AccessTypeOnlyAddress.With(creator)
+	onlyOther := types.AccessTypeOnlyAddress.With(other)
+
+	specs := map[string]struct {
+		defaultPermssion    types.AccessType
+		requestedPermission *types.AccessConfig
+		// grantedPermission is set iff no error
+		grantedPermission types.AccessConfig
+		// expError is nil iff the request is allowed
+		expError *sdkerrors.Error
+	}{
+		"override everybody": {
+			defaultPermssion:    types.AccessTypeEverybody,
+			requestedPermission: &onlyCreator,
+			grantedPermission:   onlyCreator,
+		},
+		"default to everybody": {
+			defaultPermssion:    types.AccessTypeEverybody,
+			requestedPermission: nil,
+			grantedPermission:   types.AccessConfig{Permission: types.AccessTypeEverybody},
+		},
+		"explicitly set everybody": {
+			defaultPermssion:    types.AccessTypeEverybody,
+			requestedPermission: &types.AccessConfig{Permission: types.AccessTypeEverybody},
+			grantedPermission:   types.AccessConfig{Permission: types.AccessTypeEverybody},
+		},
+		"cannot override nobody": {
+			defaultPermssion:    types.AccessTypeNobody,
+			requestedPermission: &onlyCreator,
+			expError:            sdkerrors.ErrUnauthorized,
+		},
+		"default to nobody": {
+			defaultPermssion:    types.AccessTypeNobody,
+			requestedPermission: nil,
+			grantedPermission:   types.AccessConfig{Permission: types.AccessTypeNobody},
+		},
+		"only defaults to code creator": {
+			defaultPermssion:    types.AccessTypeOnlyAddress,
+			requestedPermission: nil,
+			grantedPermission:   onlyCreator,
+		},
+		"can explicitly set to code creator": {
+			defaultPermssion:    types.AccessTypeOnlyAddress,
+			requestedPermission: &onlyCreator,
+			grantedPermission:   onlyCreator,
+		},
+		"cannot override which address in only": {
+			defaultPermssion:    types.AccessTypeOnlyAddress,
+			requestedPermission: &onlyOther,
+			expError:            sdkerrors.ErrUnauthorized,
+		},
+	}
+	for msg, spec := range specs {
+		t.Run(msg, func(t *testing.T) {
+			params := types.DefaultParams()
+			params.InstantiateDefaultPermission = spec.defaultPermssion
+			keeper.SetParams(ctx, params)
+			codeID, err := contractKeeper.Create(ctx, creator, hackatomWasm, spec.requestedPermission)
+			require.True(t, spec.expError.Is(err), err)
+			if spec.expError == nil {
+				codeInfo := keeper.GetCodeInfo(ctx, codeID)
+				require.Equal(t, codeInfo.InstantiateConfig, spec.grantedPermission)
+			}
+		})
+	}
+}
+
 func TestCreateDuplicate(t *testing.T) {
 	ctx, keepers := CreateTestInput(t, false, SupportedFeatures)
 	keeper := keepers.ContractKeeper

x/wasm/types/types.go

@@ -330,3 +330,21 @@ func VerifyAddressLen() func(addr []byte) error {
 		return nil
 	}
 }
+
+// IsSubset will return true if the caller is the same as the superset,
+// or if the caller is more restrictive than the superset.
+func (a AccessConfig) IsSubset(superSet AccessConfig) bool {
+	switch superSet.Permission {
+	case AccessTypeEverybody:
+		// Everything is a subset of this
+		return a.Permission != AccessTypeUnspecified
+	case AccessTypeNobody:
+		// Only an exact match is a subset of this
+		return a.Permission == AccessTypeNobody
+	case AccessTypeOnlyAddress:
+		// An exact match or nobody
+		return a.Permission == AccessTypeNobody || (a.Permission == AccessTypeOnlyAddress && a.Address == superSet.Address)
+	default:
+		return false
+	}
+}

x/wasm/types/types_test.go

@@ -372,3 +372,84 @@ func TestVerifyAddressLen(t *testing.T) {
 		})
 	}
 }
+
+func TestAccesConfigSubset(t *testing.T) {
+	specs := map[string]struct {
+		check    AccessConfig
+		superSet AccessConfig
+		isSubSet bool
+	}{
+		"nobody <= nobody": {
+			superSet: AccessConfig{Permission: AccessTypeNobody},
+			check:    AccessConfig{Permission: AccessTypeNobody},
+			isSubSet: true,
+		},
+		"only > nobody": {
+			superSet: AccessConfig{Permission: AccessTypeNobody},
+			check:    AccessConfig{Permission: AccessTypeOnlyAddress, Address: "foobar"},
+			isSubSet: false,
+		},
+		"everybody > nobody": {
+			superSet: AccessConfig{Permission: AccessTypeNobody},
+			check:    AccessConfig{Permission: AccessTypeEverybody},
+			isSubSet: false,
+		},
+		"unspecified > nobody": {
+			superSet: AccessConfig{Permission: AccessTypeNobody},
+			check:    AccessConfig{Permission: AccessTypeUnspecified},
+			isSubSet: false,
+		},
+		"nobody <= everybody": {
+			superSet: AccessConfig{Permission: AccessTypeEverybody},
+			check:    AccessConfig{Permission: AccessTypeNobody},
+			isSubSet: true,
+		},
+		"only <= everybody": {
+			superSet: AccessConfig{Permission: AccessTypeEverybody},
+			check:    AccessConfig{Permission: AccessTypeOnlyAddress, Address: "foobar"},
+			isSubSet: true,
+		},
+		"everybody <= everybody": {
+			superSet: AccessConfig{Permission: AccessTypeEverybody},
+			check:    AccessConfig{Permission: AccessTypeEverybody},
+			isSubSet: true,
+		},
+		"unspecified > everybody": {
+			superSet: AccessConfig{Permission: AccessTypeEverybody},
+			check:    AccessConfig{Permission: AccessTypeUnspecified},
+			isSubSet: false,
+		},
+		"nobody <= only": {
+			superSet: AccessConfig{Permission: AccessTypeOnlyAddress, Address: "owner"},
+			check:    AccessConfig{Permission: AccessTypeNobody},
+			isSubSet: true,
+		},
+		"only <= only(same)": {
+			superSet: AccessConfig{Permission: AccessTypeOnlyAddress, Address: "owner"},
+			check:    AccessConfig{Permission: AccessTypeOnlyAddress, Address: "owner"},
+			isSubSet: true,
+		},
+		"only > only(other)": {
+			superSet: AccessConfig{Permission: AccessTypeOnlyAddress, Address: "owner"},
+			check:    AccessConfig{Permission: AccessTypeOnlyAddress, Address: "other"},
+			isSubSet: false,
+		},
+		"everybody > only": {
+			superSet: AccessConfig{Permission: AccessTypeOnlyAddress, Address: "owner"},
+			check:    AccessConfig{Permission: AccessTypeEverybody},
+			isSubSet: false,
+		},
+		"nobody > unspecified": {
+			superSet: AccessConfig{Permission: AccessTypeUnspecified},
+			check:    AccessConfig{Permission: AccessTypeNobody},
+			isSubSet: false,
+		},
+	}
+
+	for name, spec := range specs {
+		t.Run(name, func(t *testing.T) {
+			subset := spec.check.IsSubset(spec.superSet)
+			require.Equal(t, spec.isSubSet, subset)
+		})
+	}
+}