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

all: address panics in BeginBlocker and EndBlocker #18623

Closed
1 task done
odeke-em opened this issue Dec 4, 2023 · 1 comment
Closed
1 task done

all: address panics in BeginBlocker and EndBlocker #18623

odeke-em opened this issue Dec 4, 2023 · 1 comment
Labels
Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Comments

@odeke-em
Copy link
Collaborator

odeke-em commented Dec 4, 2023

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

From code security scanners, there are about 30 reports of panics in BeginBlocker and EndBlocker per https://github.com/cosmos/cosmos-sdk/security/code-scanning?query=is%3Aopen+branch%3Amain+rule%3Acrypto-com%2Fcosmos-sdk-codeql%2Fbeginendblock-panic

validator, err := k.GetValidator(ctx, addr)
if err != nil {
panic(fmt.Sprintf("validator record not found for address: %X\n", addr))
}

cosmos-sdk/types/address.go

Lines 700 to 706 in f23d5c4

// cacheBech32Addr is not concurrency safe. Concurrent access to cache causes race condition.
func cacheBech32Addr(prefix string, addr []byte, cache *simplelru.LRU, cacheKey string) string {
bech32Addr, err := bech32.ConvertAndEncode(prefix, addr)
if err != nil {
panic(err)
}
if IsAddrCacheEnabled() {

// no state change
default:
panic("unexpected validator status")
}

cosmos-sdk/types/coin.go

Lines 326 to 334 in f23d5c4

if !coins.isSorted() {
panic("Coins (self) must be sorted")
}
if !coinsB.isSorted() {
panic("Wrong argument: coins must be sorted")
}
uniqCoins := make(map[string]Coin, len(coins)+len(coinsB))
// Traverse all the coins for each of the coins and coinsB.

Cosmos SDK Version

main

How to reproduce?

We need to explore those reported panics and see what we can address.

Kindly /cc-ing @elias-orijtech @julienrbrt @tac0turtle

@odeke-em odeke-em added the T:Bug label Dec 4, 2023
@github-project-automation github-project-automation bot moved this to 👀 To Do in Cosmos-SDK Dec 4, 2023
@julienrbrt julienrbrt added Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. and removed T:Bug labels Dec 4, 2023
@tac0turtle
Copy link
Member

this is covered in other issues like #12985 & #15555. This is also a known thing, even if we return an error here it will panic at a lower level. Closing this issue as it doesnt provide much information

@github-project-automation github-project-automation bot moved this from 👀 To Do to 🥳 Done in Cosmos-SDK Dec 4, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

No branches or pull requests

3 participants