-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests #19140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dling PROXY requests
@theStack you might be interested in this? |
Any chance of getting this merged? It is a bit cumbersome to wade through tons of spurious Note that this is a workaround for a bug (of mine) in the fuzzing harness -- not in |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jul 11, 2020
… in libevent when handling PROXY requests 20d31bd tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift) Pull request description: Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from bitcoin#19074 as suggested by MarcoFalke. The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness. The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path. `" http:// HTTP/1.1\n"` was a crashing input prior to this workaround. Before this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input AddressSanitizer:DEADLYSIGNAL ================================================================= ==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0) ==27905==The signal is caused by a READ memory access. ==27905==Hint: address points to the zero page. #0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37 #1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7 #2 0x55a1687f624e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/http_request.cpp:51:9 … $ echo $? 1 ``` After this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input Executed input in 0 ms *** *** NOTE: fuzzing was not performed, you have only *** executed the target code on a fixed set of inputs. *** $ echo $? 0 ``` See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) Top commit has no ACKs. Tree-SHA512: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 21, 2021
…dling PROXY requests Summary: ``` The dereference (req->evcon->http_server) takes place in evhttp_parse_request_line and is a consequence of our hacky but necessary use of the internal function evhttp_parse_firstline_ in the http_request fuzzing harness. The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path. " http:// HTTP/1.1\n" was a crashing input prior to this workaround. ``` Backport of core [[bitcoin/bitcoin#19140 | PR19140]]. Depends on D9001. Test Plan: ninja bitcoin-fuzzers echo " http:// HTTP/1.1" > input ./src/test/fuzz/http_request input Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D9002
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 17, 2022
… in libevent when handling PROXY requests 20d31bd tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift) Pull request description: Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from bitcoin#19074 as suggested by MarcoFalke. The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness. The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path. `" http:// HTTP/1.1\n"` was a crashing input prior to this workaround. Before this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input AddressSanitizer:DEADLYSIGNAL ================================================================= ==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0) ==27905==The signal is caused by a READ memory access. ==27905==Hint: address points to the zero page. #0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37 #1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7 #2 0x55a1687f624e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/http_request.cpp:51:9 … $ echo $? 1 ``` After this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input Executed input in 0 ms *** *** NOTE: fuzzing was not performed, you have only *** executed the target code on a fixed set of inputs. *** $ echo $? 0 ``` See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) Top commit has no ACKs. Tree-SHA512: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a
nullptr
dereference. Split out from #19074 as suggested by MarcoFalke.The dereference (
req->evcon->http_server
) takes place inevhttp_parse_request_line
and is a consequence of our hacky but necessary use of the internal functionevhttp_parse_firstline_
in thehttp_request
fuzzing harness.The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path.
" http:// HTTP/1.1\n"
was a crashing input prior to this workaround.Before this PR:
After this PR:
See
doc/fuzzing.md
for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the Bitcoin Core fuzzing corpus repo.Happy fuzzing :)