btcsuite
btcutil/base58: fix bug with range
The decoding was using a range over the string instead of iterating over the bytes.
Pull Request Test Coverage Report for Build 1771996489
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 79.676%
| Totals | |
|---|---|
| Change from base Build 1753768655: | 0.0% |
| Covered Lines: | 1231 |
| Relevant Lines: | 1545 |
💛 - Coveralls
Is this PR fixing the crash that occurs if you try to decode something with an emoji? What is the allowed character set here?
For example, using example-Decode, if you try to decode a string with 🤡, it’ll crash:
package main
import (
"fmt"
"github.com/btcsuite/btcd/btcutil/base58"
)
func main() {
// Decode example modified base58 encoded data.
encoded := "25JnwSn7XKfNQ🤡"
decoded := base58.Decode(encoded)
// Show the decoded data.
fmt.Println("Decoded Data:", string(decoded))
}
Result:
Output:
panic: runtime error: index out of range [10095] with length 256
goroutine 1 [running]:
github.com/btcsuite/btcd/btcutil/base58.Decode({0x49fff5, 0x10})
/tmp/gopath2191683068/pkg/mod/github.com/btcsuite/btcd/[email protected]/base58/base58.go:58 +0x395
main.main()
/tmp/sandbox2173415571/prog.go:12 +0x29
Yes, it's a fix for runes that are larger 0xFF, which does include emoji. The valid alphabet is 123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz -- i.e. base58 alphabet used by bitcoin.
Why this is not merged yet? I'm using the lib to check BTC address is valid and the issue could crash the app ;[
Similar fix downstream: https://github.com/decred/base58/blob/5b84bad1ebfd283724c012ec3e0e02f6f80b0ee2/base58.go#L25 Worth merging this PR or a direct port of that solution.
