src: fix IsIPAddress for IPv6 - #53400
Conversation
Sorry, something went wrong.
Fix the bug when copying IPv6 host to a variable to remove brackets. Fixes: nodejs#47427
| char ipv6_str[INET6_ADDRSTRLEN]; | ||
| std::copy(host.begin() + 1, host.end() - 1, ipv6_str); | ||
| ipv6_str[host.length()] = '\0'; | ||
| ipv6_str[host.length() - 2] = '\0'; |
There was a problem hiding this comment.
A better approach is to use Ada parser and validate if it is IPv6
Sorry, something went wrong.
There was a problem hiding this comment.
The IsIPAddress function is invoked by the IsAllowedHost function, which accepts a host parameter. Typically, the host represents an IP address that may include a port number. For instance, in the test case found here, the host is solely an IP address.
Upon reviewing the Ada parser, it appears to be designed primarily for parsing URLs. So, I can't use it here like this: ada::parse(host).
Is there a potential method to adapt the Ada parser for use with IP addresses?
Sorry, something went wrong.
There was a problem hiding this comment.
Since this is a bug fix, keeping the patch minimal is a good idea. Refactoring to use Ada or some entirely different validation instead does not need to happen in this PR.
Sorry, something went wrong.
tniessen
left a comment
There was a problem hiding this comment.
Good catch.
Sorry, something went wrong.
Sorry, something went wrong.
Fix the bug when copying IPv6 host to a variable to remove brackets. Fixes: nodejs#47427 PR-URL: nodejs#53400 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com>
I've investigated and found a fix to the related issue as follows:
host keeps the host address which is [::] in our case. Then, it is copied to a char array (ipv6_str) to remove brackets. However, when adding a null-terminator, the index is written wrong. The length should have been decreased by 2 to add a null terminator.
Fixes: #47427