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

Fix ExtractIPAddresses IPv6 regexp #1661

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Fix ExtractIPAddresses IPv6 regexp #1661

merged 2 commits into from
Feb 3, 2024

Conversation

BlacAmDK
Copy link
Contributor

@BlacAmDK BlacAmDK commented Dec 5, 2023

IPv6 regexp shouldn't match IPv4 address.
@a3957273 a3957273 merged commit 592745f into gchq:master Feb 3, 2024
4 checks passed
@a3957273
Copy link
Member

a3957273 commented Feb 3, 2024

Thanks, this seems like a clear improvement! :)

@n1474335
Copy link
Member

n1474335 commented Feb 6, 2024

I agree that the IPv6 regex shouldn't match a single IPv4 address on its own, which it appears it did, but it should match IPv6 dual addresses, which include an embedded IPv4 address. More info and examples here: https://www.ibm.com/docs/en/ts4500-tape-library?topic=functionality-ipv4-ipv6-address-formats

This PR removes that capability from CyberChef. Perhaps we need a bit of tweaking to match on IPv6 duals, but not individual IPv4s.

@BlacAmDK
Copy link
Contributor Author

BlacAmDK commented Feb 8, 2024

I agree that the IPv6 regex shouldn't match a single IPv4 address on its own, which it appears it did, but it should match IPv6 dual addresses, which include an embedded IPv4 address. More info and examples here: https://www.ibm.com/docs/en/ts4500-tape-library?topic=functionality-ipv4-ipv6-address-formats

This PR removes that capability from CyberChef. Perhaps we need a bit of tweaking to match on IPv6 duals, but not individual IPv4s.

After some tests, I finally found the real issue, the original regex only worked fine when it matched the whole one-line text, if it has multiple IPs in one line, it will break sometimes. for example(notice the first ip): https://gchq.github.io/CyberChef/#recipe=Extract_IP_addresses(false,true,false,false,false,false)&input=MjAwMTowZGI4OjAwMDE6MDAwMDowMDAwOjBhYjk6QzBBODowMTAyIGNhbiBiZSBjb21wcmVzc2VkIGFzIGZvbGxvd3M6IDIwMDE6ZGI4OjE6OmFiOTpDMEE4OjEwMi4

I'm not an expert on regex, don't know how to fix this issue now. Do I need to open a new pull request to revert this comment?

@n1474335
Copy link
Member

n1474335 commented Feb 9, 2024

If you could open an issue to reflect that this should be fixed, that would be great thanks!

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

Successfully merging this pull request may close these issues.

3 participants