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

internal/rand: RandStr and (*Rand).Str should have limits to their lengths lest cause memory hogs and if used can caused a Denial-of-Service #761

Closed
odeke-em opened this issue May 4, 2023 · 1 comment

Comments

@odeke-em
Copy link
Contributor

odeke-em commented May 4, 2023

Found by fuzzing and also by plain auditing, if I pass in a large integer into (*Rand).Str, like in this test

func TestRandStrHuge(t *testing.T) {
        _ = RandStr(8455134228352958140)
}

it'll consume boundless amounts of memory until it is killed by my kernel
Screen Shot 2023-05-04 at 11 14 57 PM

Screen Shot 2023-05-04 at 11 15 16 PM

Explanation

Due to its logic that just tries to get alphanumeric strings of the input length, with no length limits, it'll keep doing lots of work and consume endless amounts of memory

func (r *Rand) Str(length int) string {
chars := []byte{}
MAIN_LOOP:
for {
val := r.Int63()
for i := 0; i < 10; i++ {
v := int(val & 0x3f) // rightmost 6 bits
if v >= 62 { // only 62 characters in strChars
val >>= 6
continue
}
chars = append(chars, strChars[v])
if len(chars) == length {
break MAIN_LOOP
}
val >>= 6
}
}
return string(chars)
}

Suggestion

Please make a reasonable limit to the number of characters that can be max produced or if this function isn't used, mark it as deprecated and delete it eventually

/cc @elias-orijtech

odeke-em added a commit to orijtech/iavl that referenced this issue May 4, 2023
Adds fuzzers that have discovered some bugs, like:
* cosmos#760
* cosmos#761
odeke-em added a commit to orijtech/iavl that referenced this issue May 4, 2023
Adds fuzzers that have discovered some bugs, like:
* cosmos#760
* cosmos#761
@tac0turtle
Copy link
Member

tac0turtle commented May 5, 2023

this is used in testing, please check how its used before opening issues please

odeke-em added a commit to orijtech/iavl that referenced this issue May 24, 2023
Adds fuzzers that have discovered some bugs, like:
* cosmos#760
* cosmos#761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants