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

Newer libevent causes http_request fuzz target failure #30096

Closed
hebasto opened this issue May 13, 2024 · 9 comments
Closed

Newer libevent causes http_request fuzz target failure #30096

hebasto opened this issue May 13, 2024 · 9 comments
Labels

Comments

@hebasto
Copy link
Member

hebasto commented May 13, 2024

When building with MSVC, the libevent dependency package is provided by the vcpkg package manager.

The #27335 pinned the libevent version to 2.1.12#7 to avoid issues with the changed signature of the evhttp_connection_get_peer function.

Then, #29774 introduced the fuzz.exe binary.

It turned out that the newer libevent version 2.1.12+20230128 leads to failures in the http_request fuzz target.

To accommodate the newer libevent version, the following diff can be applied:

--- a/build_msvc/common.init.vcxproj.in
+++ b/build_msvc/common.init.vcxproj.in
@@ -90,12 +90,12 @@
       <AdditionalOptions>/utf-8 /Zc:preprocessor /Zc:__cplusplus /std:c++20 %(AdditionalOptions)</AdditionalOptions>
       <DisableSpecificWarnings>4018;4244;4267;4715;4805</DisableSpecificWarnings>
       <TreatWarningAsError>true</TreatWarningAsError>
-      <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;PROVIDE_FUZZ_MAIN_FUNCTION;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR;_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;PROVIDE_FUZZ_MAIN_FUNCTION;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <AdditionalIncludeDirectories>..\..\src;..\..\src\minisketch\include;..\..\src\univalue\include;..\..\src\secp256k1\include;..\..\src\leveldb\include;..\..\src\leveldb\helpers\memenv;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
-      <AdditionalDependencies>Iphlpapi.lib;ws2_32.lib;Shlwapi.lib;kernel32.lib;user32.lib;gdi32.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies)</AdditionalDependencies>
+      <AdditionalDependencies>bcrypt.lib;Iphlpapi.lib;ws2_32.lib;Shlwapi.lib;kernel32.lib;user32.lib;gdi32.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies)</AdditionalDependencies>
       <RandomizedBaseAddress>true</RandomizedBaseAddress>
     </Link>
   </ItemDefinitionGroup>
--- a/build_msvc/vcpkg.json
+++ b/build_msvc/vcpkg.json
@@ -15,7 +15,7 @@
   "overrides": [
     {
       "name": "libevent",
-      "version": "2.1.12#7"
+      "version": "2.1.12+20230128"
     }
   ]
 }

Here is an example of a CI log demonstrating the issue: https://github.com/hebasto/bitcoin/actions/runs/9064987376/job/24904490588:

Run http_request with args ['D:\\a\\bitcoin\\bitcoin\\src\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_seed_corpus/http_request')]
Target ['D:\\a\\bitcoin\\bitcoin\\src\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_seed_corpus/http_request')] failed with exit code 3221225477
hebasto added a commit to hebasto/bitcoin that referenced this issue May 13, 2024
@fanquake
Copy link
Member

Is there something for this repo to do here? If that patch should be applied, then this should be a PR? If not, I guess close this?

@hebasto
Copy link
Member Author

hebasto commented May 14, 2024

Is there something for this repo to do here? If that patch should be applied, then this should be a PR? If not, I guess close this?

I'm in the middle of the investigation.

@hebasto
Copy link
Member Author

hebasto commented May 14, 2024

Is there something for this repo to do here? If that patch should be applied, then this should be a PR? If not, I guess close this?

I'm in the middle of the investigation.

The crash is reproducible on Linux as well.

@hebasto hebasto changed the title windows: Newer libevent causes http_request fuzz target failure Newer libevent causes http_request fuzz target failure May 14, 2024
@fanquake
Copy link
Member

The crash is reproducible on Linux as well.

Can you post steps to reproduce. What version of libevent.

@hebasto
Copy link
Member Author

hebasto commented May 14, 2024

The crash is reproducible on Linux as well.

Can you post steps to reproduce.

Compiling with depends in this branch. Then:

FUZZ=http_request ./src/test/fuzz/fuzz /home/hebasto/git/bitcoin/qa-assets/fuzz_seed_corpus/http_request

What version of libevent.

libevent/libevent@4d85d28

@fanquake
Copy link
Member

fanquake commented May 14, 2024

Ok, then I think this can be closed. The problem here primarily seems to be that Microsoft/vcpkg is shipping unreleased code into production.

@maflcko
Copy link
Member

maflcko commented May 14, 2024

(I deleted my comment, because the fuzz CI config does not use depends, but the libevent-dev from Ubuntu)

@fanquake
Copy link
Member

Ok. Edited my comment as well. This can still be closed. Maybe an issue can be filed with vcpkg to only ship stable code in production.

@hebasto
Copy link
Member Author

hebasto commented May 14, 2024

Ok, then I think this can be closed. The problem here primarily seems to be that Microsoft/vcpkg is shipping unreleased code into production.

I agree. I opened this issue to document hebasto#199.

Closing.

@hebasto hebasto closed this as completed May 14, 2024
hebasto added a commit to hebasto/bitcoin that referenced this issue May 15, 2024
49d4c30 cmake, vcpkg: Pin `libevent` version (Hennadii Stepanov)

Pull request description:

  For more details, see bitcoin#30096.

Top commit has no ACKs.

Tree-SHA512: 38697ac78899feece1ae4dd375c7061e7a2f3ac7b1428e880f6942fc1a8614bc256c3c04b280e2841de760459826e77dce20986f86a06e8072382a4314cfa9b1
hebasto added a commit to hebasto/bitcoin that referenced this issue May 20, 2024
hebasto added a commit to hebasto/bitcoin that referenced this issue Jun 4, 2024
hebasto added a commit to hebasto/bitcoin that referenced this issue Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants