Fatal coinIssue Bug in handleMsgIssue()

[Jdkhnjggf]

Describe The Bug
The crafted message of coin's issuance leads to an arbitrary coinIssue vulnerability of the asset handler located at /x/asset/handler.go. Specifically, the handleMsgIssue() routine is designed to handle the MsgIssueCoin message in order to issue a coin. However, the sanity checks on the input message are not thorough. As a result, a malicious MsgIssueCoin message which contains inconsistent creator , symbol and amount could lead to an arbitrary coinIssue behavior, causing huge asset losses to all users. In the following, we show the related code snippet.

Code Snippets (Optional)
handleMsgIssue() /x/asset/handler.go

// handleMsgIssue Handle Msg Issue coin
func handleMsgIssue(ctx chainTypes.Context, k keeper.AssetCoinsKeeper, msg *types.MsgIssueCoin) (*sdk.Result, error) {
	... ...
	ctx.RequireAccount(msgData.Creator)

	stat, err := k.GetCoinStat(ctx.Context(), msgData.Creator, msgData.Symbol)
	if err != nil {
		return nil, sdkerrors.Wrapf(err, "get coin stat from coin %s", msg.Amount.String())
	}

	// if coins cannot be issue, if there is 1000 blocks after coin created, no one can issue
	if !stat.CanIssue && (ctx.BlockHeight() > (stat.CreateHeight + 5)) { // FIXME: for test
		return nil, sdkerrors.Wrapf(types.ErrAssetCoinCannotBeLock, "coin %s cannot be issue after 1000 block from coin create", msg.Amount.String())
	}

	if err := k.Issue(ctx.Context(), msgData.Creator, msgData.Symbol, msgData.Amount); err != nil {
		return nil, sdkerrors.Wrapf(err, "msg issue coin %s", msgData.Symbol)
	}
        ... ...
}

Issue() /x/asset/keeper/keeper.go

func (a AssetKeeper) Issue(ctx sdk.Context, creator, symbol types.Name, amount types.Coin) error {
	if err := a.issueCoinStat(ctx, amount); err != nil {
		return err
	}

	creatorAccount := types.NewAccountIDFromName(creator)
	coins, err := a.getCoins(ctx, creatorAccount)
	if err != nil {
		return sdkerrors.Wrap(err, "get coins")
	}

	if err := a.setCoins(ctx, creatorAccount, coins.Add(amount)); err != nil {
		return sdkerrors.Wrap(err, "issue set coins")
	}

	return nil
}

Input/Output

1. Craft a MsgIssueCoin : '{"creator": "kratos", "symbol": "kts", "amount": "100000kratos/kvs"}'
2. Output : update state '{"newSupply": "100000kratos/kvs", "maxSupply": "1000000kratos/kvs", "limit": "1000000kratos/kvs"}'

To Reproduce
Steps to reproduce the behavior:

1. Comment out that code in the client located at /x/asset/client/cli/issue.go (lines 52- 55)
2. make
3. sudo ./scripts/boot-testnet.sh
4. sudo ./build/ktscli tx asset create kratos kvs 1000000kratos/kvs 1 1 10 1kratos/kvs "test" --
   keyring-backend test --chain-id testing --home /testing/cli/ --from kratos
5. sudo ./build/ktscli tx asset issue kratos kts 100000kratos/kvs --keyring-backend test --chain-id
   testing --home /testing/cli/ --from kratos
6. sudo ./build/ktscli query asset coins kratos

Expected Behavior
Returns an error "coin denom should equal %s != %s, chainTypes.CoinDenom(creator, symbol), amount.GetDenom()".

Screenshots

Desktop (please complete the following information):

• OS: [macOS High Sierra 10.13.6]

Additional Context (Optional)
None

Contact Information

Email - ryzhang@peckshield.cn

[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 to get your rewards.
Thanks for your attention and contribution. Please keep trying and help us improve our chain.

Regards
KuChain Team