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.

Sign up for GitHub

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

Jump to bottom

pubkey.cpp:368:18: runtime error: implicit conversion from type 'int' of value 256 (32-bit, signed) to type 'unsigned char' changed the value to 0 (8-bit, unsigned) #25751

Closed
maflcko opened this issue Jul 30, 2022 · 1 comment · Fixed by #25642
Closed

pubkey.cpp:368:18: runtime error: implicit conversion from type 'int' of value 256 (32-bit, signed) to type 'unsigned char' changed the value to 0 (8-bit, unsigned) #25751

maflcko opened this issue Jul 30, 2022 · 1 comment · Fixed by #25642
Labels
Bug
Milestone
24.0

Comments

@maflcko
Copy link
Member

maflcko commented Jul 30, 2022

Steps to reproduce:

  • Build with integer sanitizer
  • Start: UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./src/qt/bitcoin-qt -chain=main
  • Enter getdescriptorinfo "sh(multi(1,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/5/5/5/2/8/5/8/8/8/2/2/5/8/5/2/5/85/58/5/2/5/5/5/5/2/58/2/5/8/0000/00/8/5/2/001/1/246/8/501/46/8/5/8/5/000/8/52/5/8/5/2/8/2/5/8/5/246/8/5/2/8/2/5/8/5/2/5/8/8/2/5/5/5/58/88/2/6/8/5/2/8/2/5/8/2/588/5/2/5/8/5/2/8/5/1/5/4/5/5/5/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/8/58/588/2/52/5/5/5/58/8/2/6/8/5/2/8/2/5/8/2/588/5/2/5/8/5/2/8/5/15/58/58/2/6/8/5/2/8/2/5/8/5/2/5/8/58/2/5/58/58/588/2/5/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8/2/8/5/2/8/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/8/588/2/6/8/5/2/8/2/5/5/8/5/2/0/8/5/2/00/4/5/5/588/2/6/8/5/2/8/25/5/8/2/5/58/58/8/4/1,[00000000]xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/58)0)" into the console
@maflcko maflcko added the Bug label Jul 30, 2022
@maflcko maflcko added this to the 24.0 milestone Jul 30, 2022
@darosior
Copy link
Member

darosior commented Aug 2, 2022

This is fixed by #25642.

@darosior darosior mentioned this issue Aug 2, 2022
Merged
sidhujag pushed a commit to syscoin/syscoin that referenced this issue Aug 11, 2022
@achow101
Merge bitcoin#25642: Don't wrap around when deriving an extended key …
0f6dfd1 
…at a too large depth

fb9faff extended keys: fail to derive too large depth instead of wrapping around (Antoine Poinsot)
8dc6670 descriptor: don't assert success of extended key derivation (Antoine Poinsot)
50cfc9e (pubk)key: mark Derive() as nodiscard (Antoine Poinsot)
0ca258a descriptor: never ignore the return value when deriving an extended key (Antoine Poinsot)
d3599c2 spkman: don't ignore the return value when deriving an extended key (Antoine Poinsot)

Pull request description:

  We would previously  silently wrap the derived child's depth back to `0`. Instead, explicitly fail when trying to derive an impossible depth, and handle the error in callers.

  An extended fuzzing corpus of `descriptor_parse` triggered this behaviour, which was reported by MarcoFalke.

  Fixes bitcoin#25751.

ACKs for top commit:
  achow101:
    re-ACK fb9faff
  instagibbs:
    utACK  bitcoin@fb9faff

Tree-SHA512: 9f75c23572ce847239bd15e5497df2960b6bd63c61ea72347959d968b5c4c9a4bfeee284e76bdcd7bacbf9eeb70feee85ffd3e316f353ca6eca30e93aafad343
@bitcoin bitcoin locked and limited conversation to collaborators Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
24.0
Development

Successfully merging a pull request may close this issue.

Don't wrap around when deriving an extended key at a too large depth darosior/bitcoin
2 participants
@maflcko @darosior