Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ip6address: fix parser array bounds issue/crash #413

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pallas
Copy link
Contributor

@pallas pallas commented Oct 3, 2018

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

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
Copy link
Owner

kohler commented Oct 3, 2018

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

@pallas
Copy link
Contributor Author

pallas commented Oct 3, 2018 via email

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants