Code Review: The functions 'hash5Tuple' and 'hash2Tuple' implemented incorrectly in PacketUtils.cpp.

prudens commented 3 months ago

Bug description

Describe the bug I noticed that in the 'hash5Tuple' function, when the code logic reaches the branch where 'portDst == portSrc', it compares IPv6 and IPv4 addresses directly by converting them to 'uint64_t'. The correct approach seems to be: 'memcmp(ipv6Layer->getIPv6Header()->ipDst, ipv6Layer->getIPv6Header()->ipSrc, 16) < 0'. Additionally, it should check for the parameter '!directionUnique' at the beginning to ensure the functionality is effective when 'directionUnique' is 'true'.

Code example to reproduce

in PacketUtil.cpp:202 
        if (portSrc == portDst && (uint64_t)ipv6Layer->getIPv6Header()->ipDst < (uint64_t)ipv6Layer->getIPv6Header()->ipSrc)
            srcPosition = 1;

Expected behavior

        if (!directionUnique && portSrc == portDst && memcmp(ipv6Layer->getIPv6Header()->ipDst, ipv6Layer->getIPv6Header()->ipSrc, 16) < 0)
            srcPosition = 1;

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

Clang 15

Packet capture backend (if applicable)

libpcap

seladb commented 3 months ago

@prudens good catch! This is indeed a bug, thank you for reporting it 🙏 Would you consider opening a PR with a fix?

prudens commented 3 months ago

@seladb I am very willing to submit a PR, but according to the requirements mentioned in the guidelines, my development environment is not yet properly configured to perform the necessary code checks and tests before submitting the PR.

tigercosmos commented 3 months ago

@prudens what's the difficulty for you for now?

seladb commented 3 months ago

ut according to the requirements mentioned in the guidelines, my development environment is not yet pr

@prudens I'm curious what is missing in your environment? In the following links you can find instructions on how to set up the environment based on your system:

Linux: https://pcapplusplus.github.io/docs/next/install/linux
macOS: https://pcapplusplus.github.io/docs/next/install/macos
Windows with Visual Studio: https://pcapplusplus.github.io/docs/next/install/vs
Windows with MinGW: https://pcapplusplus.github.io/docs/next/install/mingw

tigercosmos commented 3 months ago

close due to no activity.

egecetin commented 3 months ago

Is this bug fixed? If not fixed, it should remain open. Someone might want work on it even if inactive

tigercosmos commented 3 months ago

actually, let's reopen it.

prudens commented 3 months ago

I will open a PR to fix it this week.

seladb commented 2 months ago

Fixed in PR #1495

seladb / PcapPlusPlus