kohler / click

The Click modular router: fast modular packet processing and analysis
Other
735 stars 322 forks source link

ip6address: fix parser array bounds issue/crash #413

Open pallas opened 5 years ago

pallas commented 5 years ago

A malformed IP6Address, i.e. with more than 8 components, will cause the memmove/memset at the end of parsing to fail. This code is supposed to move the post-:: bytes to the end of the address and zero out the bytes in the center. Instead, it moves some of the bytes the wrong way, potentially writing prior to parts[] and then zeroing with a negative size.

Instead, make the parser stop when it hits 8 components and let parse figure out that we did not consume the entire String.

Thanks: American Fuzzy Lop

Change-Id: I2966cb22fb5b44eb9e92dfc6a94e891cf5d02fa7

kohler commented 5 years ago

I'm worried this isn't right. The bottom of IP6AddressArg::basic_parse has a bunch of d != 7 cases and 8 != 7.

pallas commented 5 years ago

The loop right above does network conversion for d+1 items. Maybe the coloncolon thing needs more refinement, I'm not sure, but I assumed the d-- if p (not currently looking at the code) would turn the 8 into a 7.

~Derrick • iPhone

On Oct 3, 2018, at 6:39 AM, Eddie Kohler notifications@github.com wrote:

I'm worried this isn't right. The bottom of IP6AddressArg::basic_parse has a bunch of d != 7 cases and 8 != 7.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.