fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSION #20995

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2101-fuzzVersion0 changing 5 files +36 −22
  1. MarcoFalke commented at 7:06 PM on January 23, 2021: member

    This fixes a fuzz bug introduced in #20881. Previously the nodes in the fuzz tests had their version initialized to a constant (PROTOCOL_VERSION). After #20881, the nodes have their version initialized to an arbitrary signed integer. This is problematic for several reasons:

    • Both nVersion and m_greatest_common_version may be initialized to 0. If a version message is processed, this leads to a crash, because m_greatest_common_version must be INIT_PROTO_VERSION while the version message is processed. See #20138
    • The "valid" range for nVersion is [MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()] (see check in net_processing)
    • The "valid" range for m_greatest_common_version is std::min(nVersion, PROTOCOL_VERSION) (see net_processing)

    Fix all issues by initializing nVersion and m_greatest_common_version to their valid ranges.


    The crashers, if someone wants to try this at home:

    ( echo 'dmVyc2lvbgAWFhYWFhYWFhYWFhYWFhYWFhYWFhZp/29uAPX//xYWFhYWFhYWFhYWFhYWFhYWFhYW
    FhYWFhYWaW9uAOr1//8WFhYWFha0ZXJzaW9uAPX//wAAAAAAABAAAAAAAAAAAAC0ZXJzaW9uAPX/
    /wBPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT08AAAAAABAAAAAAAAAAAAAAAAAA
    AAAAAAAAAAAAAAAAAAAAAAAAAAAACgAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
    AAAAAAAAAAAAAAB2ZXJzaW9uAACDJIO9vXYKAAAAAAAAAAAAAAAAAAAAAAB2ZfS1qmu1qhUVFWs=' | base64 --decode > /tmp/a ) && FUZZ=process_message_version ./src/test/fuzz/fuzz /tmp/a
    
    ( echo 'dmVyc2lvbgD//wAhTmiqN///NDcAAACENDL/iv//8DYAAHL///////79/RtcAJqamhqa/QEAAAD/
    ///+/f1oZWFkZXJzAAAAAM8BAAAAIAYibkYRGgtZyq8SaGVhZGVycwAAAAD/NDcAAACENDL/iv//
    8DYAAHL///////79/RtcAJqamhqa/QEAAAD////+/f1oZWFkZXJzAAAAAM8BAAAAIAYibkYRGgtZ
    yq8SaGVhZGVycwAAAADPAQAAACAGIm5GERoLWS1wb3J061u/KMNPOkwFXqZ///b5IgIAAD+5ubkb
    XD5hZGRyAJqamhqasP0BAAAAAAAAAP0BAAAAIf39/R0dHQAAAAAAMgAA///7//+gXqZ///b5IgIA
    AD+5ubm5ubm5AAAAAAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAFgAAAAAAAAAAAAlBmv39/f1/f39B
    f39hZGRyAG5vAACaLgAdGzY2zwEAAAAgBiJuRhEaC1ktcG9ydOtbvyjDTzpMBV6mf//2+SICAAA/
    ubm5G1w+YWRkcgCampoamrD9AQAAAAAAAAD9AQAAACH9/f0dHR0AAAAAADIAAP//+///oF6mf//2
    +SICAAA/ubm5ubm5uQAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAABYAAAAAAAAAAAAJQZr9/f39
    f39/QX9/YWRkcgBubwAAmi4AHRs2NjY2NjY2NjYCAgI2NgIA/f39/f39Nv39/TUmABxc' | base64 --decode > /tmp/b ) && FUZZ=process_message_version ./src/test/fuzz/fuzz /tmp/b
    
  2. fuzz: move-only FillNode implementation to cpp file
    This allows to modify the implementation without having to recompile all
    fuzz targets.
    
    Can be reviewed with --color-moved=dimmed-zebra
    fa99e33aeb
  3. fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSION fad3d7625a
  4. DrahtBot added the label Build system on Jan 23, 2021
  5. MarcoFalke removed the label Build system on Jan 23, 2021
  6. MarcoFalke added the label Tests on Jan 23, 2021
  7. DrahtBot commented at 9:26 PM on January 23, 2021: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #20758 (net-processing refactoring -- lose globals, move implementation details from .h to .cpp by ajtowns)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  8. practicalswift commented at 8:56 PM on January 26, 2021: contributor

    cr ACK fad3d7625aa1c2b6c343946e709e87e7168f9d9d

    Thanks for fixing this: I keep hitting this assertion failure all the time :)

  9. MarcoFalke merged this on Jan 28, 2021
  10. MarcoFalke closed this on Jan 28, 2021

  11. sidhujag referenced this in commit aa277234c6 on Jan 28, 2021
  12. MarcoFalke deleted the branch on Apr 26, 2021
  13. DrahtBot locked this on Aug 18, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 06:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me