urlapi: avoid index underflow for short ipv6 hostnames #4389
Conversation
If the input hostname is "[",
hlen will underflow to max of size_t when it is subtracted with 2.
hostname[hlen] will then cause a warning by ubsanitizer:
runtime error: addition of unsigned offset to 0x<snip> overflowed to 0x<snip>
I think that in practice, the generated code will work, and the output
of hostname[hlen] will be the first character "[".
This can be demonstrated by the following program (tested in both clang and gcc,
with -O3)
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
int main() {
char* hostname=strdup("[");
size_t hlen = strlen(hostname);
hlen-=2;
hostname++;
printf("character is %d\n",+hostname[hlen]);
free(hostname-1);
}
I found this through fuzzing, and even if it seems harmless, the proper
thing is to return early with an error.
| @@ -598,6 +598,8 @@ static CURLUcode hostname_check(struct Curl_URL *u, char *hostname) | |||
| if(hostname[0] == '[') { | |||
| char dest[16]; /* fits a binary IPv6 address */ | |||
| const char *l = "0123456789abcdefABCDEF:."; | |||
| if(hlen <= 2) | |||
There was a problem hiding this comment.
I clicked approve and then immediately it struck me. I can't think of any IPv6 address shorter than 3 letters (::1) so maybe this can abort some illegal strings and be < 5 ?
There was a problem hiding this comment.
Sounds reasonable, but why is the entire code block not wrapped in the #ifdef ENABLE_IPV6 ? I guess the correct reply if enable_ipv6 is false, is to reject anything starting with[ anyway?
There was a problem hiding this comment.
Well, first that's a really rare configuration these days so I'm not sure we need to bother too much about that, but possibly more importantly the URL parser API should probably be able to extract host names even if curl itself can't actually connect to that host.
|
Thanks! |
See
curl/lib/urlapi.c
Lines 596 to 604 in a89aeb5
If the input hostname is "[",
hlen will underflow to max of size_t when it is subtracted with 2 at line 602.
hostname[hlen] at line 604 will then cause a warning by ubsanitizer:
runtime error: addition of unsigned offset to 0x........ overflowed to 0x............
I think that in practice, the generated code will work, and the output
of hostname[hlen] will be the first character "[".
This can be demonstrated by the following program (tested in both clang and gcc,
with -O3)
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
int main() {
char* hostname=strdup("[");
size_t hlen = strlen(hostname);
hlen-=2;
hostname++;
printf("character is %d\n",+hostname[hlen]);
free(hostname-1);
}
I found this through fuzzing, and even if it seems harmless, the proper
thing is to return early with an error.