Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulnerability Report - No cost price submission due to lack of error checking #612

Closed
Hellobloc opened this issue Jun 15, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@Hellobloc
Copy link

Hellobloc commented Jun 15, 2023

No cost price submission due to lack of error checking

Hello, UnUniFi Project Party. Through a quick code read, I may have found a vulnerability in your project.

Introduction

In the following code content, the error handling of x.bankKeeper.SendCoins is missing, which will result in a malicious user being able to set the status without cost.
https://github.com/UnUniFi/chain/blob/v3.1.0/app/app.go

func (k Keeper) ValidateAuthorityAndDeposit(ctx sdk.Context, marketId string, address sdk.AccAddress, deposit sdk.Coin) error {
  params := k.GetParams(ctx)
  if deposit.IsLT(params.DepositForPosting) {
    k.bankKeeper.SendCoinsFromAccountToModule(ctx, address, types.ModuleName, sdk.NewCoins(deposit))
    return sdkerrors.Wrapf(types.ErrInvalidOracle, "price deposit %s is less than minimum price deposit %s", deposit, params.DepositForPosting)
  }

  _, err := k.GetOracle(ctx, marketId, address)
  if err != nil {
    k.bankKeeper.SendCoinsFromAccountToModule(ctx, address, types.ModuleName, sdk.NewCoins(deposit))
    return err
  }

Impact

This vulnerability can lead to no-cost price submissions, ultimately resulting in almost $0 price for any nft

Related issue code

https://github.com/UnUniFi/chain/blob/v3.1.0/x/pricefeed/keeper/keeper.go#L49-L63
Issues that can be used for reference
ignite/cli#2828

Credit to

https://twitter.com/MaLucasBC

@Hellobloc Hellobloc added the bug Something isn't working label Jun 15, 2023
@Hellobloc Hellobloc changed the title Vulnerability Report Vulnerability Report - No cost price submission due to lack of error checking Jun 16, 2023
@kimurayu45z
Copy link
Contributor

Thank you for reporting.
This code is

  • listed up internally for improvement
  • not linked in app.go

so currently it doesn't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants