NULL Pointer Dereference in handleMsgIssue()

[Jdkhnjggf]

Describe The Bug

The issuance of an unregistered coin leads to a null pointer dereference 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 checks on the input message are not thorough. As a result, a malicious MsgIssueCoin message can be crafted to contain an unregistered coin and its execution could lead to a null pointer dereference of the running processes. In the following, we show the related code snippet.

Code Snippets (Optional)

89	// handleMsgIssue Handle Msg Issue coin
90	func handleMsgIssue(ctx chainTypes.Context, k keeper.AssetCoinsKeeper, msg *types.MsgIssueCoin) (*sdk.Result, error) {
	    ... ...
105	    stat, err := k.GetCoinStat(ctx.Context(), msgData.Creator, msgData.Symbol)
106	    if err != nil {
107	        return nil, sdkerrors.Wrapf(err, "get coin stat from coin %s", msg.Amount.String())
108	    }
109	
110	    // if coins cannot be issue, if there is 1000 blocks after coin created, no one can issue
111	    if !stat.CanIssue && (ctx.BlockHeight() > (stat.CreateHeight + 5)) { // FIXME: for test
112	        return nil, sdkerrors.Wrapf(types.ErrAssetCoinCannotBeLock, "coin %s cannot be issue after 1000 block from coin create", msg.Amount.String())
113	    }

Input/Output

1. Craft a MsgIssueCoin: '{"creator": "kratos", "symbol": "kvs", "amount": "1kratos/kvs"}'
2. Output: '{"panic": "runtime error: invalid memory address or nil pointer dereference"}'

To Reproduce

Steps to reproduce the behavior:

1. sudo ./scripts/boot-testnet.sh
2. sudo ./build/ktscli tx asset issue kratos kvs 1kratos/kvs --keyring-backend test --chain-id testing --home /testing/cli/ --from kratos

Expected Behavior

Returns an error "coin stat is nil".

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 with such detail information!

We have tested the issue you mentioned and did reproduce it.

Note that issue #7 and issue #8 were caused by the same reason, where function GetCoinStat() didn't return an error message as expect when query token is not existed, which resulted in an exception thrown by the subsequent handing process. This is our negligence. Thanks for your reminder.

We count the two reports(#7 and #8) as one since they were caused by the same reason. What's more, we believe the impact that might be delivered by the issue you mentioned is rather limited, it will not affect the normal operation of the chain, and it is not in the scope from P1 to P4. After evaluation, we consider it is not a valid vulnerability but it does a good improve suggestion.

Thanks for your attention and contribution! Please keep trying and help us improve our chain.

Regards

KuChain Team