fuzz: http_request workaround for libevent < 2.1.1 #18682

1. 
   theStack commented at 12:20 pm on April 17, 2020: member

   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 https://github.com/libevent/libevent/commit/8ac3c4c25bea4b9948ab91cd00605bf34fc0bd72 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_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.

2. fanquake added the label Tests on Apr 17, 2020
3. 
   laanwj commented at 12:27 pm on April 17, 2020: member

   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. I guess the only reason that this is acceptable is because this is not production code but only a fuzzer. Thanks for the workaround anyhow, this makes sure xenial with libevent 2.0.21 can compile the fuzzers. (though I wouldn’t actually recommend spending time fuzzing libevent 2.0.21, it’s 8 years old)
4. in src/test/fuzz/http_request.cpp:35 in a9f657ca6e outdated

   29+inline int evhttp_parse_headers_(struct evhttp_request* r, struct evbuffer* b)
   30+{
   31+    return evhttp_parse_headers(r, b);
   32+}
   33+#else
   34 extern "C" int evhttp_parse_firstline_(struct evhttp_request*, struct evbuffer*);
   

   ---

   ---

   

   MarcoFalke commented at 1:22 pm on April 17, 2020:

   cc @practicalswift Can this fuzzer be adjusted to remove evhttp_parse_firstline_?

   ---

   

   practicalswift commented at 2:27 pm on April 17, 2020:

   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.

   ---

   

   laanwj commented at 3:08 pm on April 17, 2020:

   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.

   ---

   

   practicalswift commented at 3:16 pm on April 17, 2020:

   @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:

   0    HTTPRequest http_request{evreq, true};
   1    const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod();
   2    (void)RequestMethodString(request_method);
   3    (void)http_request.GetURI();
   4    (void)http_request.GetHeader("Host");
   

5. 
   MarcoFalke commented at 3:40 pm on April 17, 2020: member

   Travis fails, so this can’t be merged in its current form
6. theStack force-pushed on Apr 17, 2020
7. 
   theStack commented at 4:19 pm on April 17, 2020: member

   Travis fails, so this can’t be merged in its current form

   My fault, I used an internal define from event-config.h for checking the version (named differently later) instead of LIBEVENT_VERSION_NUMBER that is defined in event.h. Force-pushed.

8. fuzz: http_request workaround for libevent < 2.1.1

   Before libevent 2.1.1, internal functions names didn't end with an underscore.

   6f8b498d18
9. theStack force-pushed on Apr 17, 2020
10. 
    theStack commented at 6:51 pm on April 17, 2020: member

    Travis still fails in two instances due to functional tests not passing, (obviously) unrelated to this change.
11. 
    hebasto commented at 6:52 pm on April 17, 2020: member

    Travis still fails in two instances due to functional tests not passing, (obviously) unrelated to this change.

    Restarted.

12. hebasto approved
13. 
    hebasto commented at 7:59 pm on April 17, 2020: member

    ACK 6f8b498d186df5aa08dbb9ca8fdeab6652f1db5e, tested on xenial:

    0$ ./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=clang-8 CXX=clang++-8
    1$ make
    

14. MarcoFalke merged this on Apr 17, 2020
15. MarcoFalke closed this on Apr 17, 2020

    ---

16. sidhujag referenced this in commit 1a2b0e8de6 on Apr 18, 2020
17. theStack deleted the branch on Dec 1, 2020
18. Fabcien referenced this in commit 1684b6761b on Jan 21, 2021
19. DrahtBot locked this on Feb 15, 2022

Contributors
theStack laanwj MarcoFalke practicalswift hebasto

Labels
Tests