-
Notifications
You must be signed in to change notification settings - Fork 682
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
Bug in IDnsResource::decodeName. #35
Comments
Similar problem in SSLClientHelloMessage::SSLClientHelloMessage: |
Can you please provide pcap files that throw these exceptions? |
Add to the line https://github.com/seladb/PcapPlusPlus/blob/master/Packet%2B%2B/src/DnsLayer.cpp#L47 string "std::cout << "offsetInLayer: " << (int) offsetInLayer << std::endl;". Then use attached file. |
Hex string of the wrong udp packet (make UdpPacket.dat) : |
Could you please send me the code you're running or the stack trace of the exception? |
I simply made UdpPacket.dat with content that I wrote above and launched Pcap++Examples.PacketParsing: Program received signal SIGSEGV, Segmentation fault. My OS is CentOS 7-2.1511. |
OK, let me check that and come back to you |
Fixed the DnsLayer bug. You said there's also a problem with SSLClientHelloMessage. Could you please provide a pcap file and the steps to reproduce the exception? |
You have corrected bug is not entirely. Here's the output from the GDB: The function 'pcpp::IDnsResource::decodeName' loops. |
In the functions for working with SSL (e.g. SSLClientHelloMessage::getSessionIDLength, SSLClientHelloMessage::getCipherSuiteCount) never checked obtained values. As a result, there is going beyond the borders of the packet: |
I made a simple application and ran it on the live traffic. After some time, the application is terminated due to exhausting all available memory. The problem in the code of parsing packets for each layer. |
When you're saying that the problem in the code of parsing packets for each layer - do you mean the bugs in DNS and SSL parsing you mentioned? Or do you think there are other bugs as well? |
For the experiment, I disabled SSL and DNS layers. The first potential problem in the line 367 in the file HttpLayer.cpp: using strlen is not safe. I think that you must use strnlen, like: m_FieldSize = strnlen(fieldData,m_HttpMessage->m_DataLen); |
Since I don't have the testing environment that you have it's difficult for me to reproduce the bugs you encounter. I'm currently working on the DNS bug, then I'll move to the SSL bug and then to the HTTP bug. I'd really appreciate if you can help me verify the bugfixes and find more bugs. Also, I'd appreciate if you can send me pcap files that reproduce these bugs. I will add tests to Packet++ unit-test with these files |
I hope I fixed the 3 issues you found (DNS, SSL and HTTP). Please tell me if they are really fixed and if you find more. In addition, I'd appreciate if you can send me pcap files that reproduce these bugs. I will add tests to Packet++ unit-test with these files |
I think that in the HTTP layer strchr must be replaced by memchr. |
I can't make pcap files, because files are very big, I launch application on 10gbe interface. |
Bugfix #35 - strchr replaced by memchr in the HTTP layer
Thanks for the pull request, I merged it into master branch. Did you verify my fixes in DNS, SSL and HTTP layers? Do you still see crashes or memory leaks in those or other layers? |
HTTP layer worke fine. DNS layer still have problem: |
It's hard to understand from this stack trace what's the exception and why it occurs. Can you please send me a pcap file or some other information so I can reproduce it in my environment? |
SSLlayer is work as expected. |
I've attached dump of dns packets: |
I can't reproduce the segmentation fault with the last packet in the pcap file. I tried running PcapPrinter and Pcap++Examples.PacketParsing with this packet on both Windows 7 and Ubuntu 12.04 32-bit but no success in reproducing it. Can you share your code and OS? |
I launch DpdkTrafficFilter application from the examples on the mirror of real traffic. Program receives ~ 130000 packets per second. |
I don't have CentOS 7 and I can simulate DpdkTrafficFilter only on Ubuntu VM. I'll try to reproduce it, hope I'll be able to. Can you please try to debug the problem and suggest a solution? |
I'll try to reproduce the issue and find the source of its origin. |
Try packet on Pcap++Examples.PacketParsing: |
I think that in function DnsLayer::parseResources you must check numOfQuestions and numOfAnswers for valid values, for an example these values can't be more then 16. This will reduce the processing time of non valid DNS packets. |
You're right, I'll add this code. In addition I found the problem in the packet you sent - there is an endless loop there. I'll fix that either |
Fixed that issue, please tell me if you see more segmentation faults or memory leaks |
Please notify me if everything is ok so I can close the bug |
I launched a test application during ~ 8 hours and was not observed problems. |
If non-dns packet gets into DnsLayer, then we will get SIGSEGV, because function size_t IDnsResource::decodeName not checking going beyond the limits of the packet.
The text was updated successfully, but these errors were encountered: