search

tdex-network / tdex-daemon

Go implementation of the TDEX Beta Daemon
https://tdex.network
MIT License
11 stars 13 forks source link

/market/price insufficent checks resulting in invalid memory address or nil pointer dereference #746

Open pirinskodxb opened 8 months ago

pirinskodxb commented 8 months ago

Reproduce: TDEXD running with a market created with no deposits (perhaps also when funds used up) to either base or quote asset.

An unopen market can still be accessed directly via the base/quote params to /market/price and still trigger the issue although most public clients adhere to the markets within /markets/.

hitting /market/price will result in nil pointer dereference.

/tdex/fun/tdex-daemon/internal/core/application/trade/service.go:92 +0x1d2

baseAssetBalance := balance[mkt.BaseAsset].GetConfirmedBalance() log.Debug(baseAssetBalance) quoteAssetBalance := balance[mkt.QuoteAsset].GetConfirmedBalance() log.Debug(quoteAssetBalance)

altafan commented 8 months ago

@pirinskodxb Thanks for spotting this out.

I was a bit scared because I thought the daemon was panicking, but once I reproduced, I realized that it's returning a non-formatted error to the client.

This can be easily fixed by checking that the market is open before returning its price.

Screenshot 2024-02-28 at 14 03 53
pirinskodxb commented 8 months ago

I think the grpc thread does panic but the daemon handles it. I had to mod the UnaryServerInterceptor() to get the backtrace. Perhaps markets shouldn't be openable without sufficent funds.

Users could potentially bypass checks at /markets/ since all they need are asset ID's which are public of course, not sure what potential ramifications that could have but now I will figure out how to make a trade and see what else I discover :)

altafan commented 8 months ago

Indeed. Will create a ticket for this. Thanks.