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
Base58 decoding is done without checking that the input size is reasonable #17501
Comments
I would say it's not the encoder/decoder responsibility to check input sizes. Good generic code works for any input size. But of course, the application side (e.g. address parsing routines) could have a check to see if inputs are reasonable. They have that knowledge. |
@laanwj Yes, that information would need to be either a.) provided by the caller for enforcement in Also the documentation of |
In any case, I think the surprising thing here, for most users of strings encodings such as Base64, is that decode time doesn't scale linearly with size. It would make sense to document that. |
5909bcd Add bounds checks in key_io before DecodeBase58Check (Pieter Wuille) 2bcf1fc Pass a maximum output length to DecodeBase58 and DecodeBase58Check (Pieter Wuille) Pull request description: Fixes bitcoin#17501. ACKs for top commit: laanwj: code review ACK 5909bcd practicalswift: ACK 5909bcd -- code looks correct Tree-SHA512: 4807f4a9508dee9c0f1ad63f56f70f4ec4e6b7e35eb91322a525e3da3828521a41de9b8338a6bf67250803660b480d95fd02ce6b2fe79c4c88bc19b54f9d8889
5909bcd Add bounds checks in key_io before DecodeBase58Check (Pieter Wuille) 2bcf1fc Pass a maximum output length to DecodeBase58 and DecodeBase58Check (Pieter Wuille) Pull request description: Fixes bitcoin#17501. ACKs for top commit: laanwj: code review ACK 5909bcd practicalswift: ACK 5909bcd -- code looks correct Tree-SHA512: 4807f4a9508dee9c0f1ad63f56f70f4ec4e6b7e35eb91322a525e3da3828521a41de9b8338a6bf67250803660b480d95fd02ce6b2fe79c4c88bc19b54f9d8889
5909bcd Add bounds checks in key_io before DecodeBase58Check (Pieter Wuille) 2bcf1fc Pass a maximum output length to DecodeBase58 and DecodeBase58Check (Pieter Wuille) Pull request description: Fixes bitcoin#17501. ACKs for top commit: laanwj: code review ACK 5909bcd practicalswift: ACK 5909bcd -- code looks correct Tree-SHA512: 4807f4a9508dee9c0f1ad63f56f70f4ec4e6b7e35eb91322a525e3da3828521a41de9b8338a6bf67250803660b480d95fd02ce6b2fe79c4c88bc19b54f9d8889
5909bcd Add bounds checks in key_io before DecodeBase58Check (Pieter Wuille) 2bcf1fc Pass a maximum output length to DecodeBase58 and DecodeBase58Check (Pieter Wuille) Pull request description: Fixes bitcoin#17501. ACKs for top commit: laanwj: code review ACK 5909bcd practicalswift: ACK 5909bcd -- code looks correct Tree-SHA512: 4807f4a9508dee9c0f1ad63f56f70f4ec4e6b7e35eb91322a525e3da3828521a41de9b8338a6bf67250803660b480d95fd02ce6b2fe79c4c88bc19b54f9d8889
5909bcd Add bounds checks in key_io before DecodeBase58Check (Pieter Wuille) 2bcf1fc Pass a maximum output length to DecodeBase58 and DecodeBase58Check (Pieter Wuille) Pull request description: Fixes bitcoin#17501. ACKs for top commit: laanwj: code review ACK 5909bcd practicalswift: ACK 5909bcd -- code looks correct Tree-SHA512: 4807f4a9508dee9c0f1ad63f56f70f4ec4e6b7e35eb91322a525e3da3828521a41de9b8338a6bf67250803660b480d95fd02ce6b2fe79c4c88bc19b54f9d8889
5909bcd Add bounds checks in key_io before DecodeBase58Check (Pieter Wuille) 2bcf1fc Pass a maximum output length to DecodeBase58 and DecodeBase58Check (Pieter Wuille) Pull request description: Fixes bitcoin#17501. ACKs for top commit: laanwj: code review ACK 5909bcd practicalswift: ACK 5909bcd -- code looks correct Tree-SHA512: 4807f4a9508dee9c0f1ad63f56f70f4ec4e6b7e35eb91322a525e3da3828521a41de9b8338a6bf67250803660b480d95fd02ce6b2fe79c4c88bc19b54f9d8889
Base58 decoding is currently done without checking that the input size is reasonable.
This can lead to excessive decoding run time if an attacker can control the base58 input being decoded.
DecodeBase58/DecodeBase58Check(…)
run time sampled with varying input sizes:DecodeBase58/DecodeBase58Check(…)
is reachable via the RPC interface using the following code paths:Other code paths involving base58-decoding:
The text was updated successfully, but these errors were encountered: