pellegre / libcrafter

A high level C++ network packet sniffing and crafting library.
298 stars 88 forks source link

Issue BitFields::Read for bitfields that span an octet #51

Closed martind1111 closed 7 years ago

martind1111 commented 7 years ago

When trying crafting IPv6 fragments, I had issues crafting the fragment offset of IPv6FragmentationHeader when fragment offset was greater than 31.

Here is some sample code:

include

include <netinet/ip6.h>

Crafter::IPv6FragmentationHeader ipv6_frag_header; struct ip6_frag ip6frag; ip6frag.ip6f_nxt = 6; ip6frag.ip6f_offlg = ntohs(255 << 3) | 1); ip6frag.ip6f_ident = ntohl(1234); ipv6_fragf_header.PutData((const Crafter::byte *) &ip6frag, sizeof(ip6frag)); printf(("%u\n", ipv6_frag_header.GetFrameOffset());

Output of this program is not 255 as expected.

I narrowed down the issue to the last line of BitField::Read (line 136 of Fields/BitsField.h). The current line is: human = ntohl(value >> rightMargin);

I believe the line should read instead: human = ntohl(value) >> rightMargin;

oliviertilmans commented 7 years ago

Hi Martin,

This is likely related to #47 (which I had completely forgotten about ...). Hopefully I should be able to dedicate some time to finally solve this.

martind1111 commented 7 years ago

I bet that doing the bit shifting as I recommend outside of the ntohl instead of inside of the ntohl will fix the issue for MPLS as well.

Martin

On Mon, Apr 3, 2017 at 8:18 AM, Olivier Tilmans notifications@github.com wrote:

Hi Martin,

This is likely related to #47 https://github.com/pellegre/libcrafter/issues/47 (which I had completely forgotten about ...). Hopefully I should be able to dedicate some time to finally solve this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pellegre/libcrafter/issues/51#issuecomment-291126336, or mute the thread https://github.com/notifications/unsubscribe-auth/AFHLORVSg9uYIANpS_aC7zLufO1XtOOoks5rsOOGgaJpZM4MtSHP .

oliviertilmans commented 7 years ago

Could you check if https://github.com/pellegre/libcrafter/commit/2102a27af776418d02eaa99439604cad28e5e893 solves this for you ?

It seems to have fixed issues on my side but it has been so long I am afraid I broke other stuff ...

martind1111 commented 7 years ago

I appreciate the followup. I'll check it out tomorrow at work.

Martin

On Mon, May 22, 2017 at 11:25 AM, Olivier Tilmans notifications@github.com wrote:

Could you check if 2102a27 https://github.com/pellegre/libcrafter/commit/2102a27af776418d02eaa99439604cad28e5e893 solves this for you ?

It seems to have fixed issues on my side but it has been so long I am afraid I broke other stuff ...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pellegre/libcrafter/issues/51#issuecomment-303133547, or mute the thread https://github.com/notifications/unsubscribe-auth/AFHLOTmOJY4qOwpYoua2WdHzHK_7dVRPks5r8aj_gaJpZM4MtSHP .

oliviertilmans commented 7 years ago

Looking back at your example, libcrafter parses ip6f_offlg as 3 separate fields:

This is consistent with the definition of ip6_frag which aggregates all 3 in an uint16_t. See the appropriate Get* calls or simply call ipv6_frag_header.Print(); ipv6_frag_header.HexDump(std::cout);

martind1111 commented 7 years ago

I tested your patch and it fixes the issue I reported.

Martin

On Mon, May 22, 2017 at 11:25 AM, Olivier Tilmans notifications@github.com wrote:

Could you check if 2102a27 https://github.com/pellegre/libcrafter/commit/2102a27af776418d02eaa99439604cad28e5e893 solves this for you ?

It seems to have fixed issues on my side but it has been so long I am afraid I broke other stuff ...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pellegre/libcrafter/issues/51#issuecomment-303133547, or mute the thread https://github.com/notifications/unsubscribe-auth/AFHLOTmOJY4qOwpYoua2WdHzHK_7dVRPks5r8aj_gaJpZM4MtSHP .