Fatal coinIssue Bug in handleMsgIssue() #9
Description
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
- Craft a MsgIssueCoin : '{"creator": "kratos", "symbol": "kts", "amount": "100000kratos/kvs"}'
- Output : update state '{"newSupply": "100000kratos/kvs", "maxSupply": "1000000kratos/kvs", "limit": "1000000kratos/kvs"}'
To Reproduce
Steps to reproduce the behavior:
- Comment out that code in the client located at /x/asset/client/cli/issue.go (lines 52- 55)
- make
- sudo ./scripts/boot-testnet.sh
- 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 - sudo ./build/ktscli tx asset issue kratos kts 100000kratos/kvs --keyring-backend test --chain-id
testing --home /testing/cli/ --from kratos - 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 - [email protected]