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.

Sign up for GitHub

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

Jump to bottom

Added bounds checking. #1939

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added bounds checking. #1939

wants to merge 1 commit into from
+12 −0

Conversation

chadf
Copy link
Contributor

@chadf chadf commented Jul 3, 2023

Found multiple buffer over-reads while fuzzing.

==23556==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x614000000dcc at pc 0x000000dd8ce7 bp 0x7fffffffc3f0 sp 0x7fffffffbbc0
READ of size 2 at 0x614000000dcc thread T0
    #0 0xdd8ce6 in __asan_memcpy /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
    #1 0xf6bdbf in buf16toh(void const*) /projects/i2pd/build/../libi2pd/I2PEndian.h:96:2
    #2 0xf39d6a in bufbe16toh(void const*) /projects/i2pd/build/../libi2pd/I2PEndian.h:116:9
    #3 0x11c59b7 in i2p::data::LeaseSet2::ReadMetaLS2TypeSpecificPart(unsigned char const*, unsigned long) /projects/i2pd/libi2pd/LeaseSet.cpp:452:28
    #4 0x11be7a5 in i2p::data::LeaseSet2::ReadFromBuffer(unsigned char const*, unsigned long, bool, bool) /projects/i2pd/libi2pd/LeaseSet.cpp:355:9
    #5 0x11b8026 in i2p::data::LeaseSet2::LeaseSet2(unsigned char, unsigned char const*, unsigned long, bool, unsigned short) /projects/i2pd/libi2pd/LeaseSet.cpp:290:4
    #6 0xe0eb34 in LLVMFuzzerTestOneInput /projects/i2pd/fuzz/fuzz-LeaseSet2.cc:33:11
    #7 0xd59593 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #8 0xd458e2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #9 0xd4a942 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
    #10 0xd5f192 in main /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10

==23615==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x614002291fcc at pc 0x000000dd8ce7 bp 0x7fffffffbf30 sp 0x7fffffffb700
READ of size 2 at 0x614002291fcc thread T0
    #0 0xdd8ce6 in __asan_memcpy /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
    #1 0xf6bdbf in buf16toh(void const*) /projects/i2pd/build/../libi2pd/I2PEndian.h:96:2
    #2 0xf39d6a in bufbe16toh(void const*) /projects/i2pd/build/../libi2pd/I2PEndian.h:116:9
    #3 0x11c3f4d in i2p::data::LeaseSet2::ReadStandardLS2TypeSpecificPart(unsigned char const*, unsigned long) /projects/i2pd/libi2pd/LeaseSet.cpp:398:28
    #4 0x11be598 in i2p::data::LeaseSet2::ReadFromBuffer(unsigned char const*, unsigned long, bool, bool) /projects/i2pd/libi2pd/LeaseSet.cpp:352:9
    #5 0x11b8026 in i2p::data::LeaseSet2::LeaseSet2(unsigned char, unsigned char const*, unsigned long, bool, unsigned short) /projects/i2pd/libi2pd/LeaseSet.cpp:290:4
    #6 0xe0eb34 in LLVMFuzzerTestOneInput /projects/i2pd/fuzz/fuzz-LeaseSet2.cc:33:11
    #7 0xd59593 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #8 0xd58d7a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3
    #9 0xd59ee9 in fuzzer::Fuzzer::MutateAndTestOne() /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19
    #10 0xd5a8a5 in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, std::__1::allocator<fuzzer::SizedFile> >&) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5
    #11 0xd4a825 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6
    #12 0xd5f192 in main /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10

==23578==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x621000068d68 at pc 0x0000010c1f12 bp 0x7fffffffbdf0 sp 0x7fffffffbde8
READ of size 1 at 0x621000068d68 thread T0
    #0 0x10c1f11 in i2p::data::ByteStreamToBase64(unsigned char const*, unsigned long, char*, unsigned long) /projects/i2pd/libi2pd/Base.cpp:113:12
    #1 0x11ee19f in i2p::data::NetDb::HandleDatabaseSearchReplyMsg(std::__1::shared_ptr<i2p::I2NPMessage const>) /projects/i2pd/libi2pd/NetDb.cpp:930:13
    #2 0xe0ecdf in LLVMFuzzerTestOneInput /projects/i2pd/fuzz/fuzz-NetDb-HandleDatabaseSearchReplyMsg.cc:29:19
    #3 0xd596d3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #4 0xd45a22 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #5 0xd4aa82 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
    #6 0xd5f2d2 in main /usr/src/contrib/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10

r4sas
r4sas requested changes Jul 4, 2023
View reviewed changes
libi2pd/NetDb.cpp
@@ -922,6 +922,10 @@ namespace data
else if(!m_FloodfillBootstrap)
LogPrint (eLogWarning, "NetDb: Requested destination for ", key, " not found");

// All peers hashs in buffer?
if(msg->GetPayloadLength() < (size_t) (33 + num * 32))
Copy link
Member

@r4sas r4sas Jul 4, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use constant instead of 32.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the constant name for a router identity hash? I don't see anything already defined in any headers.

I just copied the values from with-in the for() loop below it.

                for (int i = 0; i < num; i++)
                {
                        const uint8_t * router = buf + 33 + i*32;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no suggestions are given, will add new IDENTITY_HASH_SIZE constant to Identity.h.

@orignal
Copy link
Contributor

orignal commented Jul 5, 2023

#3 0x11c59b7 in i2p::data::LeaseSet2::ReadMetaLS2TypeSpecificPart(unsigned char const*, unsigned long

This part looks more interesting then bounds checking itself. Meaning that somebody is publishing Meta LeaseSet. What for?

@chadf
Copy link
Contributor Author

chadf commented Jul 5, 2023 •
edited

This part looks more interesting then bounds checking itself. Meaning that somebody is publishing Meta LeaseSet. What for?

Not necessarily. It was found using fuzzing (sending random input to LeaseSet2()), which can be done via a Database Store Message. So someone could craft a leaseset2 message for hostile purposes, e.g. causing a crash//DoS, or maybe extract internal data (assuming some code path makes that possible).

@chadf chadf mentioned this pull request Jul 5, 2023
Open
@chadf
Copy link
Contributor Author

chadf commented Jul 5, 2023

This has been replaced by pull request #1941, with new commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r4sas r4sas r4sas requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@chadf @orignal @r4sas