PcapPlusPlus icon indicating copy to clipboard operation
PcapPlusPlus copied to clipboard

Crash when handling malformed packet

Open bratbiswas opened this issue 10 months ago • 10 comments

Bug description

Describe the bug IPReassembly::processPacket(...) function crashes when handling a malformed packet. The payload size indicated in the packet is incorrect - it is far larger than what is actually present. The code does not have a check for correctness of the payload size value in the IP header. That leads to the problem.

I am having trouble adding the PCAP for you to test. Let me know if I can send it through alternate means. Here is the hex dump of the offending packet:

0000 ac 85 3d b9 fe 0d ac 85 3d b9 fd e4 88 47 00 00 ..=.....=....G.. 0010 8f 01 4f a6 0f 01 01 04 00 24 00 1d f7 e3 00 00 ..O......$...... 0020 00 01 00 00 00 01 ff 03 c0 21 09 14 00 08 43 3e .........!....C> 0030 f2 1d 00 00 00 00 00 00 00 00 00 00 ............

bad-packet.tar.gz

Code example to reproduce

void process(pcpp::RawPacket &raw)
{
	pcpp::IPReassembly                     reassembler ;
	pcpp::IPReassembly::ReassemblyStatus   status ;
	pcpp::Packet                           parsedPacket(raw, pcpp::UDP) ;
	pcpp::Packet                          *assembled = reassembler.processPacket(&parsedPacket, status) ;
	:
	:
	:
	:
}

Expected behavior IP reassembler should return MALFORMED_FRAGMENT.

PcapPlusPlus versions tested on

PcapPlusPlus master branch

Other PcapPlusPlus version (if applicable)

No response

Operating systems tested on

Linux

Other operation systems (if applicable)

No response

Compiler version

GCC 12.3.0

Packet capture backend (if applicable)

libpcap

bratbiswas avatar May 14 '25 06:05 bratbiswas

The code snippet I gave should be enough to reproduce the problem. The real application code I use, obviously, does not create and throw away the reassembler for every packet :-)

I have fixed the problem on my setup and I can proceed with this. I'll attach it here - in case you want to take a look.

IPReassembly.tar.gz

bratbiswas avatar May 14 '25 06:05 bratbiswas

Thank you @bratbiswas for reporting this issue! Would you consider opening a PR with the fix?

seladb avatar May 15 '25 06:05 seladb

I would have loved to. But I am not very conversant with github and might end up messing up things. Should I try? Will you be able to undo if I make mistakes?

bratbiswas avatar May 15 '25 08:05 bratbiswas

Sure, I encourage you to try! Don't worry, you won't break or mess up anything 🙂

If you want to know how to fork your repo and open a pull request, simply ask ChatGPT! 🤩

Image

Or if you prefer the more traditional way, you can read about it here: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

seladb avatar May 15 '25 09:05 seladb

Thanks for the encouragement. I did try and was about to create the pull request when I realized that you want me to fork the dev branch and not the master branch. I forked the master branch and committed and pushed :-) As I said, I am not very conversant with github and I didn't even know of the existence of the branches. The more I explore, the more I learn.

Would you want me to fork the dev branch and repeat the process? Or should I go ahead with the pull request on this fork?

Sorry about the screw up.

bratbiswas avatar May 21 '25 06:05 bratbiswas

I'm not sure it matters if you fork master or dev, as long as you submit your PR to be merged to dev. If I'm wrong and it doesn't work I guess you'll have to fork dev

seladb avatar May 21 '25 07:05 seladb

@seladb I pushed the changes and created a pull request. Hope you got a notification?

bratbiswas avatar May 24 '25 11:05 bratbiswas

@seladb I pushed the changes and created a pull request. Hope you got a notification?

@bratbiswas thanks for checking in! I left this comment: https://github.com/seladb/PcapPlusPlus/pull/1829#issuecomment-2900178506, not sure if you noticed

seladb avatar May 25 '25 07:05 seladb

SoS ...SoS ...

I wrote the test case, built and tested. I then did 'git add' for the files and then a 'git commit' for them. But the 'git push' is failing. I am out of my depth here.

This was my command: $ git push origin fix-malformed-pkt-crash

To github.com-personal:bratbiswas/PcapPlusPlus.git ! [rejected] fix-malformed-pkt-crash -> fix-malformed-pkt-crash (non-fast-forward) error: failed to push some refs to 'github.com-personal:bratbiswas/PcapPlusPlus.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

I tried 'git fetch' and 'git pull' and then attempted 'git push' again. No luck.

Sorry ... I really don't know much about git.

bratbiswas avatar May 28 '25 11:05 bratbiswas

SoS ...SoS ...

I wrote the test case, built and tested. I then did 'git add' for the files and then a 'git commit' for them. But the 'git push' is failing. I am out of my depth here.

This was my command: $ git push origin fix-malformed-pkt-crash

To github.com-personal:bratbiswas/PcapPlusPlus.git ! [rejected] fix-malformed-pkt-crash -> fix-malformed-pkt-crash (non-fast-forward) error: failed to push some refs to 'github.com-personal:bratbiswas/PcapPlusPlus.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details.

I tried 'git fetch' and 'git pull' and then attempted 'git push' again. No luck.

Sorry ... I really don't know much about git.

I put this comment in GitHub Copilot and here is it's output 😄

It looks like your push is being rejected because your local branch is behind the remote branch. To resolve this, try the following steps:

  1. Make sure all your local changes are committed.
  2. Pull the latest changes from the remote branch with rebase:
git pull --rebase origin fix-malformed-pkt-crash

If there are conflicts, resolve them as prompted and continue with:

git rebase --continue

Once the rebase is complete, push your changes:

git push origin fix-malformed-pkt-crash

If you run into any errors or need more help, please share the exact error message here and I’ll be happy to assist further.

seladb avatar May 29 '25 07:05 seladb

This bug was fixed in https://github.com/seladb/PcapPlusPlus/pull/1829. Thank you so much @bratbiswas for reporting this issue and providing a fix, much appreciated! 🙏 💓

seladb avatar Jun 12 '25 03:06 seladb