fuzz: http_request workaround for libevent < 2.1.1 #18682
Conversation
|
It's kind of bizarre that we're calling private functions in the first place—there's no guarantee they keep existing at all or with the same signature which we hard-code here. |
| { | ||
| return evhttp_parse_headers(r, b); | ||
| } | ||
| #else | ||
| extern "C" int evhttp_parse_firstline_(struct evhttp_request*, struct evbuffer*); |
There was a problem hiding this comment.
cc @practicalswift Can this fuzzer be adjusted to remove evhttp_parse_firstline_?
There was a problem hiding this comment.
Not that I know of but if someone can come up with a less "bizarre" solution than the one I wrote that would be awesome :) We should avoid calling private functions if possible.
There was a problem hiding this comment.
I think ideally, fuzzing libevent's internals belongs in libevent and not in our code. But I don't think it's a big problem to have it like this.
There was a problem hiding this comment.
@laanwj Just to clarify: the reason for the calls to the internal functions is not to fuzz libevent's internals but to be able to do HTTPRequest http_request{evreq, true}; where the evhttp_request object is initialized/created in a way resembling the real-world attack surface. Let me know if you can think of a way to do that without calling internal functions - that would be great but I couldn't find a way to do it :)
Context:
HTTPRequest http_request{evreq, true};
const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod();
(void)RequestMethodString(request_method);
(void)http_request.GetURI();
(void)http_request.GetHeader("Host");
|
Travis fails, so this can't be merged in its current form |
a9f657c to
b75a128
Compare
My fault, I used an internal define from |
Before libevent 2.1.1, internal functions names didn't end with an underscore.
b75a128 to
6f8b498
Compare
|
Travis still fails in two instances due to functional tests not passing, (obviously) unrelated to this change. |
Restarted. |
6f8b498 fuzz: http_request workaround for libevent < 2.1.1 (Sebastian Falbesoner) Pull request description: The fuzz test `http_request` calls the following two internal libevent functions: * `evhttp_parse_firstline_` * `evhttp_parse_headers_` Before libevent 2.1.1 however, internal functions names didn't end with an underscore (see libevent commit libevent/libevent@8ac3c4c and [Changelog for 2.1.1.-alpha](https://github.com/libevent/libevent/blob/master/ChangeLog#L1830) when the change was first mentioned) hence the build fails with a linking error. This PR adds a preprocessor workaround to the test that checks for the libevent version (via ~`_EVENT_NUMERIC_VERSION`~ `LIBEVENT_VERSION_NUMBER`) and creates wrapper functions mapping to naming scheme without underscore in case the version is older than 2.1.1. Tested with Ubuntu Xenial 16.04.6 LTS and clang-8. ACKs for top commit: hebasto: ACK 6f8b498, tested on xenial: Tree-SHA512: 3b9e0147b8aea22e417d418e3b6d4905f5be131c2b0ae4b0f8b9411c5606d2e22f1b23e1ecc6980ecab907c61404de09e588aae1ac43cf70cf9e8d006bbdee73
Summary: ``` Before libevent 2.1.1, internal functions names didn't end with an underscore. ``` Backport of core [[bitcoin/bitcoin#18682 | PR18682]]. Depends on D9000. Test Plan: ninja bitcoin-fuzzers I can't test the fix since I have no system with an old enough libevent that match our minimum requirements. Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Differential Revision: https://reviews.bitcoinabc.org/D9001
The fuzz test
http_requestcalls the following two internal libevent functions:evhttp_parse_firstline_evhttp_parse_headers_Before libevent 2.1.1 however, internal functions names didn't end with an underscore (see libevent commit libevent/libevent@8ac3c4c and Changelog for 2.1.1.-alpha when the change was first mentioned) hence the build fails with a linking error.
This PR adds a preprocessor workaround to the test that checks for the libevent version (via
_EVENT_NUMERIC_VERSIONLIBEVENT_VERSION_NUMBER) and creates wrapper functions mapping to naming scheme without underscore in case the version is older than 2.1.1.Tested with Ubuntu Xenial 16.04.6 LTS and clang-8.