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

fix: fix vuln demonstrated by #583 #584

Merged
merged 8 commits into from
Sep 22, 2023

Commits on Apr 10, 2023

  1. fix vuln demonstrated by gnolang#583

    Original fix at cosmos/iavl#582, is simply:
    
    ```
    if len(pin.Left) > 0 && len(pin.Right) > 0 {
    	return nil, errors.New("both left and right child hashes are set")
    }
    ```
    Our iavl functions however don't return errors. Proposing to use
    `panic()` instead, as it does in other parts of this file.
    
    More about this 2022-10-07 vuln:
    
    https://medium.com/@Beosin_com/how-did-the-bnb-chain-exploiter-pass-iavl-proof-verification-an-in-depth-analysis-by-beosin-c925b77bc13e
    https://twitter.com/buchmanster/status/1578879225574350848
    grepsuzette committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    d7bcd68 View commit details
    Browse the repository at this point in the history
  2. import original iavl proof_forgery_test.go 2022-10-08

    Won't compile yet.
    Is a test of the iavl proof forgery as exploited during BSC 2022-10-07 hack.
    
    Notes:
    1. proof_forgery_test.go comes from cosmos/iavl#582
    2. gist showing the same vuln at https://gist.github.com/samczsun/8635f49fac0ec66a5a61080835cae3db
    
    The test is not going to compile as is, it needs some work.
    grepsuzette committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    2074287 View commit details
    Browse the repository at this point in the history
  3. proof_forgery_test.go compiles, and fails the test as expected (-> vu…

    …lnerable)
    
    output follows
    
    --- FAIL: TestProofForgery (0.00s)
        proof_forgery_test.go:69:
                    Error Trace:    /home/bob/opt/src/COINS/Cosmos/GNO/gno/pkgs/iavl/proof_forgery_test.go:69
                    Error:          Should be empty, but was [73 209 82 89 222 179 131 99 170 27 180 58 80 20 211 (...) 94 7 254 45 183 20 244]
                    Test:           TestProofForgery
                    Messages:       roothash must be empty if both left and right are set
    FAIL
    FAIL    command-line-arguments  0.003s
    FAIL
    grepsuzette committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    fc32b17 View commit details
    Browse the repository at this point in the history
  4. Test now passes whenever gnolang#583 is applied

    and fails otherwise (fails when vuln not fixed)
    
    It should now be ok for review.
    grepsuzette committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    81cac3b View commit details
    Browse the repository at this point in the history
  5. make test pass

    grepsuzette committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    f65e6ce View commit details
    Browse the repository at this point in the history
  6. remove a useless panic

    applying @zikovicmilos suggestion
    grepsuzette committed Apr 10, 2023
    Configuration menu
    Copy the full SHA
    7e835d6 View commit details
    Browse the repository at this point in the history

Commits on Jun 1, 2023

  1. Configuration menu
    Copy the full SHA
    e4a0544 View commit details
    Browse the repository at this point in the history

Commits on Sep 22, 2023

  1. Configuration menu
    Copy the full SHA
    f4766e5 View commit details
    Browse the repository at this point in the history