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

[Bug] Weirdness in dlt_en10mb_merge_layer3 if alignment is enforced (L7 fuzzing test failure) #726

Closed
cbiedl opened this issue Apr 21, 2022 · 1 comment
Assignees
Labels

Comments

@cbiedl
Copy link

cbiedl commented Apr 21, 2022

This was previously mentioned in #725 but some research suggests it's a bigger issue. It seems something goes horribly wrong in fuzzing if alignment is enforced.

How to repeat:

  • Rebuild on an architecture where alignment is enforced, or on any architecture after manually poking unaligned_cv_fail=yes into configure.ac l.1772.
  • Run this fuzzing test: ./src/tcprewrite -i test/test.pcap --fuzz-seed=41 --fuzz-factor=2 -o /dev/null --verbose -d 3

Expected: Test passes

Observed:

Fatal Error in tcprewrite.c:main() line 146:
 Error rewriting packets: From edit_packet.c:fix_ipv4_checksums() line 74:
Invalid packet: Expected IPv4 packet: got 15: pkt=1

After hacking some extra code that dumps the content of packet before and after invoking plugin_merge_layer3 in fuzzing.c around line 309, the following output was seen:

Before:

0000 00 1f f3 3c e1 13 f8 1e-df e5 84 3a 08 00 45 00 ...<.......:..E.
0010 00 4f de 53 40 00 40 06-47 ab ac 10 0b 0c 4a 7d .O.S@.@.G.....J}
0020 13 11 fc 35 01 bb c6 d9-14 d0 c5 1e 2d bf 80 18 ...5........-...
0030 ff ff cb 8c 00 00 01 01-08 0a 1a 7d 84 2c 37 c5 ...........}.,7.
0040 58 b0 15 03 01 00 16 43-1a 88 1e fa 7a bc 22 6e X......C....z."n

After:

0000 00 1f f3 3c e1 13 f8 1e-df e5 84 3a 08 00 fc 35 ...<.......:...5
0010 01 bb c6 d9 14 d0 c5 1e-2d bf 80 18 ff ff cb 8c ........-.......
0020 00 00 01 01 08 0a 1a 7d-84 2c 37 c5 58 b0 15 03 .......}.,7.X...
0030 01 00 16 ff ff ff ff ff-ff ff ff ff ff ff ff ff ................
0040 ff 00 a7 5d cc 64 ea 8e-92 00 00 00 00 00 00 00 ...].d..........

(Using a visual differ like vimdiff or meld is recommended.)

So the modified l4 buffer is copied into offset 0xd while it obviously should arrive at offset 0x22. Subsequently, the sanity checks before re-computing find invalid data and bail out. This smells like a confusion between l3 and l4 but quite frankly I fail to see how to fix this.

Possibly this was just recently introduced, in

commit cc4def9d2278164014d6cbd8532e448d561d0cb6
Author: Fred Klassen <fklassen@appneta.com>
Date:   Sun Jan 30 10:52:58 2022 -0800

    Feature #563 IPv6 support for plugin_merge_layer3()

... which would explain why this was no problem in the past.

Related, I'd like to suggest a ./configure option like "--force-alignment" so issues like these could easily be detected during a (special) build on any platform.

@fklassen fklassen self-assigned this Apr 22, 2022
@fklassen fklassen added the bug label Apr 22, 2022
@fklassen
Copy link
Member

fklassen commented Aug 6, 2022

Fixed in #725.

@fklassen fklassen closed this as completed Aug 6, 2022
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

2 participants