KuChainNetwork / Project-Deluge

Identifying technical vulnerabilities
6 stars 2 forks source link

TransferBurn Bug in Transfer() #10

Open Jdkhnjggf opened 4 years ago

Jdkhnjggf commented 4 years ago

Describe The Bug The self-transfer of any accounts lead to an unexpected coinsBurn of the asset handler located at /x/asset/keeper/keeper.go. Specifically, the Transfer() routine is designed to handle the KuTransfMsg message in order to transfer coins. However, the checks on the input message are not thorough. As a result, an accidental KuTransfMsg message which contains the same from and to account could lead to an unexpected coinsBurn behavior, causing asset losses to that user. In the following, we show the related code snippet.

Code Snippets (Optional)

159 func (a AssetKeeper) Transfer(ctx sdk.Context, from, to types.AccountID, amount types.Coins) error {
        ... ...
180     fromCoins, err := a.getCoins(ctx, from)
181     if err != nil {
182         return sdkerrors.Wrap(err, "get from coins")
183     }
184
185     toCoins, err := a.getCoins(ctx, to)
186     if err != nil {
187         return sdkerrors.Wrap(err, "get to coins")
188     }
189
190     coinSubed, hasNeg := fromCoins.SafeSub(amount)
191     if hasNeg {
192         return sdkerrors.Wrap(types.ErrAssetCoinNoEnough, "transfer")
193     }
194
195     if err := a.checkIsCanUseCoins(ctx, from, amount, fromCoins); err != nil {
196         return sdkerrors.Wrap(err, "transfer")
197     }
198
199     if err := a.setCoins(ctx, to, toCoins.Add(amount...)); err != nil {
200         return sdkerrors.Wrap(err, "set to coins")
201     }
202
203     if err := a.setCoins(ctx, from, coinSubed); err != nil {
204         return sdkerrors.Wrap(err, "set from coins")
205     }

Input/Output

  1. Craft a KuTransfMsg: '{"from":"kratos","to":"kratos","amount":[{"denom":"kratos/kts","amount":"100000000"}]}'
  2. Output: None

To Reproduce Steps to reproduce the behavior:

  1. sudo ./scripts/boot-testnet.sh
  2. sudo ./build/ktscli query asset coins kratos
  3. sudo ./build/ktscli tx asset transfer kratos kratos kratos 100000000kratos/kts --keyring-backend test --chain-id testing --home /testing/cli/ --from kratos
  4. sudo ./build/ktscli query asset coins kratos

Expected Behavior Returns an error "from account cannot be equal to to account".

Screenshots

self-transfer1 self-transfer2

Desktop (please complete the following information):

Additional Context (Optional) None

Contact Information

Email - ryzhang@peckshield.cn

Pisces-Anjou commented 4 years ago

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 P3. 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