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

Fuzzing PcapPlusPlus about packet file analyzing #1263

Open
Hyeong-Uk opened this issue Dec 14, 2023 · 16 comments
Open

Fuzzing PcapPlusPlus about packet file analyzing #1263

Hyeong-Uk opened this issue Dec 14, 2023 · 16 comments
Labels

Comments

@Hyeong-Uk
Copy link

I'm a student of Seoul National University(Korea), and I tried to fuzz this library(only for file analyzing, not for analyzing live communication) for exercise.. I think I found heap overflow and few different minor weak points... Please check it and report some details to me.

I used PcapPlusPlus v23.09 on Linux(Ubuntu 20.04LTS 64bit) and AFL fuzzer 2.57b.
Test code: test1.txt
There are Initial test pcap file and crashed test file but can't upload because github issue doesn't support .pcap uploading...
If you want inputs, contact me with kohowo1999@snu.ac.kr or kohowo2000@gmail.com(recommended) .

Crashes: crash.log
The crash id of heap buffer overflow is 42.

@seladb
Copy link
Owner

seladb commented Dec 14, 2023

Thank you @Hyeong-Uk for reporting this issue!

Maybe you can upload the pcap file with a different extension (i.e .txt)?

Also - would you consider providing a fix to this issue?

@seladb seladb added the fuzzing label Dec 14, 2023
@Hyeong-Uk
Copy link
Author

Hyeong-Uk commented Dec 14, 2023

@seladb Here are pcap files:
Initial pcap input(afl fuzzing): datafin3.txt
Crashed pcap input: id:000042,sig:11,src:000000,op:flip2,pos:57.txt

+Sorry for (can)not providing a fix
+Would you report me about results? I'm curious whether it is really a weak point or not.

Thank you.

@seladb
Copy link
Owner

seladb commented Dec 15, 2023

Thank you @Hyeong-Uk ! We'll comment on this ticket once we have a fix

cc @sashashura

@sashashura
Copy link
Contributor

@Hyeong-Uk Could you provide your harness?

@tigercosmos
Copy link
Collaborator

@Hyeong-Uk is this still an issue? or I will close the ticket.

@Hyeong-Uk
Copy link
Author

@sashashura I'm very sorry for seeing your comment now. I'm just a novice for this field, so I don't understand what does 'provide harness' mean. Does it mean 'provide my execution environment'?
@tigercosmos Could you delay closing a little bit? Or if you.. judge that this issue is meaningless, you can close the ticket, of course.

@sashashura
Copy link
Contributor

Hi @Heysunk, harness is the code you used to trigger the crash. I must apologize, I didn't notice the test1.txt first. This is what I meant. We have a backlog of security issues to fix, but it seems the item 42 with this stack is not in the list of already known issues:

==1942299==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000000b19 at pc 0x0000004c0dda bp 0x7ffecbca39f0 sp 0x7ffecbca31b8
READ of size 256 at 0x611000000b19 thread T0
    #0 0x4c0dd9 in __asan_memcpy (/home/hyeonguk/SNU4_2/opensrc/PcapPlusPlus/examples_bin/test1+0x4c0dd9)
    #1 0x6820ce in pcpp::PayloadLayer::PayloadLayer(unsigned char const*, unsigned long, bool) /home/hyeonguk/SNU4_2/opensrc/PcapPlusPlus/Packet++/src/PayloadLayer.cpp:14:2
    #2 0x51a39b in splitIPPacketToFragmentsBySize(pcpp::RawPacket*, unsigned long, pcpp::PointerVector<pcpp::RawPacket>&) /home/hyeonguk/SNU4_2/opensrc/PcapPlusPlus/Examples/test1/test1.cpp:891:22
    #3 0x51b9a6 in processPacketsIPF(pcpp::IFileReaderDevice*, pcpp::IFileWriterDevice*, int, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, std::map<unsigned short, bool, std::less<unsigned short>, std::allocator<std::pair<unsigned short const, bool> > >, bool, FragStats&) /home/hyeonguk/SNU4_2/opensrc/PcapPlusPlus/Examples/test1/test1.cpp:993:4
    #4 0x545eda in main /home/hyeonguk/SNU4_2/opensrc/PcapPlusPlus/Examples/test1/test1.cpp:2353:2
    #5 0x7fd362bae082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #6 0x44924d in _start (/home/hyeonguk/SNU4_2/opensrc/PcapPlusPlus/examples_bin/test1+0x44924d)

Could you please send the initial pcap and the test file that triggers the crash to my email (listed in my profile)?

@Hyeong-Uk
Copy link
Author

Hyeong-Uk commented May 2, 2024

@sashashura Hello, I has been busy for preparing graduate school in SNU(Seoul National University(Korea)), and fortunately my acceptance to graduate school was decided. And now that I have some free time, I'm going to focus on this work. I sent those files to your email, so if you are still interested in this issue, please check and report me whether it is really a weak point or not(and if it is real weak point, then how crash occured in detail.). Thank you very much.

@tigercosmos
Copy link
Collaborator

@Hyeong-Uk I think you can put the files here for more people to review. (simply click the "Paste, drop, or click to add files" button)

@Hyeong-Uk
Copy link
Author

@tigercosmos Thank you for your comment. ^^ However, all the basic files are here(above), for example: test code(test1.txt), crash log(crash.log), fuzzed initial pcap file(datafin3.txt), etc.(and also, environment information.). Though, anyone who are interested in this issue can request me for additional files or information - then I will send them to your email.

  • For here, please understand me putting .txt files <- because I can't upload original files due to github policy.

@sashashura
Copy link
Contributor

Hi @Hyeong-Uk, we fully understand the reason for changing file extensions. This is very common practice.

It seems there was confusion from my side which file is which, so I'll just iterate to be sure we have all we need:

Optional files that are not strictly needed to reproduce the crash:

I'll look into it when time permits.

@Hyeong-Uk
Copy link
Author

@sashashura Thanks for your comment, that's all true. I'll look forward to your report. Thanks.

@tigercosmos
Copy link
Collaborator

@sashashura Is there any update?

@egecetin
Copy link
Collaborator

egecetin commented Nov 5, 2024

Are we sure that this issue really caused by the library itself? I checked the code of PayloadLayer constructor, but the constructor only copies the provided data (using pointer + length). So, the only reason for overflow should be either wrong/misaligned pointer or length. And the code (test1.txt Line 891) directly calls this constructor so there is no internal call from other layer. Fuzzer code calls it with this pointer and length directly. Am i missing something @sashashura @seladb @tigercosmos? What do you think?

@sashashura
Copy link
Contributor

I agree, the issue in case 42 is that the size of m_Data of ipLayer is 11, but curFragSize == 256 is passed to the pcpp::PayloadLayer constructor.
However, the code is copy/paste from Examples/IPFragUtil, so it raises a question should we fix the example.

@seladb
Copy link
Owner

seladb commented Nov 8, 2024

In that case we can fix the example 👍

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

5 participants