From 76cd9390da2fc40cbd344d6d8d83e1e3460ac350 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 25 May 2021 10:37:44 +0200 Subject: [PATCH 1/2] Fix str2wire ipv6hint like ipv4hint was fixed --- sldns/str2wire.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sldns/str2wire.c b/sldns/str2wire.c index 05a623a2d..1ed41764d 100644 --- a/sldns/str2wire.c +++ b/sldns/str2wire.c @@ -1087,7 +1087,7 @@ static int sldns_str2wire_svcbparam_ipv6hint(const char* val, uint8_t* rd, size_t* rd_len) { int count; - char ip_str[INET_ADDRSTRLEN+1]; + char ip_str[INET6_ADDRSTRLEN+1]; char *next_ip_str; uint32_t *ip_wire_dst; size_t i; @@ -1113,10 +1113,11 @@ sldns_str2wire_svcbparam_ipv6hint(const char* val, uint8_t* rd, size_t* rd_len) while (count) { if (!(next_ip_str = strchr(val, ','))) { - if (inet_pton(AF_INET, val, rd + *rd_len) != 1) - *rd_len += LDNS_IP6ADDRLEN; + if (inet_pton(AF_INET6, val, rd + *rd_len) != 1) break; + *rd_len += LDNS_IP6ADDRLEN; + assert(count == 1); } else if (next_ip_str - val >= (int)sizeof(ip_str)) @@ -1125,12 +1126,11 @@ sldns_str2wire_svcbparam_ipv6hint(const char* val, uint8_t* rd, size_t* rd_len) else { memcpy(ip_str, val, next_ip_str - val); ip_str[next_ip_str - val] = 0; - if (inet_pton(AF_INET, ip_str, rd + *rd_len) != 1) { - *rd_len += LDNS_IP6ADDRLEN; - + if (inet_pton(AF_INET6, ip_str, rd + *rd_len) != 1) { val = ip_str; /* to use in error reporting below */ break; } + *rd_len += LDNS_IP6ADDRLEN; val = next_ip_str + 1; } From 5f22f3a9cf108903ef107ef1a147674ce637bdde Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 25 May 2021 10:41:00 +0200 Subject: [PATCH 2/2] Shift data pointer when scanning svcb wire data Also make internal auxilary functions static --- sldns/wire2str.c | 97 +++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/sldns/wire2str.c b/sldns/wire2str.c index 0d41a2bb8..a0d3ee3e9 100644 --- a/sldns/wire2str.c +++ b/sldns/wire2str.c @@ -957,19 +957,18 @@ sldns_print_svcparamkey(char** s, size_t* slen, uint16_t svcparamkey) } } -int sldns_wire2str_svcparam_port2str(char** s, - size_t* slen, uint16_t data_len, uint16_t data) +static int sldns_wire2str_svcparam_port2str(char** s, + size_t* slen, uint16_t data_len, uint8_t* data) { int w = 0; if (data_len != 2) return -1; /* wireformat error, a short is 2 bytes */ - w = sldns_str_print(s, slen, "=%d", (int)data); + w = sldns_str_print(s, slen, "=%d", (int)sldns_read_uint16(data)); return w; } -static int -sldns_wire2str_svcparam_ipv4hint2str(char** s, +static int sldns_wire2str_svcparam_ipv4hint2str(char** s, size_t* slen, uint16_t data_len, uint8_t* data) { char ip_str[INET_ADDRSTRLEN + 1]; @@ -998,7 +997,7 @@ sldns_wire2str_svcparam_ipv4hint2str(char** s, return w; } -int sldns_wire2str_svcparam_ipv6hint2str(char** s, +static int sldns_wire2str_svcparam_ipv6hint2str(char** s, size_t* slen, uint16_t data_len, uint8_t* data) { char ip_str[INET6_ADDRSTRLEN + 1]; @@ -1029,7 +1028,7 @@ int sldns_wire2str_svcparam_ipv6hint2str(char** s, return w; } -int sldns_wire2str_svcparam_mandatory2str(char** s, +static int sldns_wire2str_svcparam_mandatory2str(char** s, size_t* slen, uint16_t data_len, uint8_t* data) { int w = 0; @@ -1051,7 +1050,7 @@ int sldns_wire2str_svcparam_mandatory2str(char** s, return w; } -int sldns_wire2str_svcparam_alpn2str(char** s, +static int sldns_wire2str_svcparam_alpn2str(char** s, size_t* slen, uint16_t data_len, uint8_t* data) { uint8_t *dp = (void *)data; @@ -1088,7 +1087,7 @@ int sldns_wire2str_svcparam_alpn2str(char** s, return w; } -int sldns_wire2str_svcparam_ech2str(char** s, +static int sldns_wire2str_svcparam_ech2str(char** s, size_t* slen, uint16_t data_len, uint8_t* data) { int size; @@ -1119,70 +1118,66 @@ int sldns_wire2str_svcparam_ech2str(char** s, int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* slen) { uint16_t svcparamkey, data_len; - uint8_t* data = *d; int written_chars = 0; + int r; - if(*dlen < 4) return 0; /* verify that we actualy have data */ + /* verify that we have enough data to read svcparamkey and data_len */ + if(*dlen < 4) + return -1; - svcparamkey = sldns_read_uint16(data); + svcparamkey = sldns_read_uint16(*d); + data_len = sldns_read_uint16(*d+2); + *d += 4; + *dlen -= 4; + + /* verify that we have data_len data */ + if (data_len > *dlen) + return -1; written_chars += sldns_print_svcparamkey(s, slen, svcparamkey); - - // (*dlen) -= written_chars; - - // @TODO fix this to be dynamic and correct - // fprintf(stderr, "*dlen2: %zu\n", *dlen); - // fprintf(stderr, "data_len %zu\n", data_len); - (*dlen) = 0; - - data_len = sldns_read_uint16(data+2); - - // if (size != val_len + 4) - // return 0; wireformat error - - // if (!data_len) { - // /* Some SvcParams MUST have values */ - // switch (svcparamkey) { - // case SVCB_KEY_ALPN: - // case SVCB_KEY_PORT: - // case SVCB_KEY_IPV4HINT: - // case SVCB_KEY_IPV6HINT: - // case SVCB_KEY_MANDATORY: - // return 0; - // default: - // return 1; - // } - // } - + if (!data_len) { + /* Some SvcParams MUST have values */ + switch (svcparamkey) { + case SVCB_KEY_ALPN: + case SVCB_KEY_PORT: + case SVCB_KEY_IPV4HINT: + case SVCB_KEY_IPV6HINT: + case SVCB_KEY_MANDATORY: + return -1; + default: + return written_chars; + } + } switch (svcparamkey) { case SVCB_KEY_PORT: - written_chars += sldns_wire2str_svcparam_port2str(s, slen, data_len, sldns_read_uint16(data+4)); + r = sldns_wire2str_svcparam_port2str(s, slen, data_len, *d); break; case SVCB_KEY_IPV4HINT: - written_chars += sldns_wire2str_svcparam_ipv4hint2str(s, slen, data_len, data+4); + r = sldns_wire2str_svcparam_ipv4hint2str(s, slen, data_len, *d); break; case SVCB_KEY_IPV6HINT: - written_chars += sldns_wire2str_svcparam_ipv6hint2str(s, slen, data_len, data+4); + r = sldns_wire2str_svcparam_ipv6hint2str(s, slen, data_len, *d); break; case SVCB_KEY_MANDATORY: - written_chars += sldns_wire2str_svcparam_mandatory2str(s, slen, data_len, data+4); + r = sldns_wire2str_svcparam_mandatory2str(s, slen, data_len, *d); break; case SVCB_KEY_NO_DEFAULT_ALPN: - return 0; /* wireformat error, should not have a value */ + return -1; /* wireformat error, should not have a value */ case SVCB_KEY_ALPN: - written_chars += sldns_wire2str_svcparam_alpn2str(s, slen, data_len, data+4); + r = sldns_wire2str_svcparam_alpn2str(s, slen, data_len, *d); break; case SVCB_KEY_ECH: - written_chars += sldns_wire2str_svcparam_ech2str(s, slen, data_len, data+4); + r = sldns_wire2str_svcparam_ech2str(s, slen, data_len, *d); break; default: break; } - - // @TODO set str_len to 0: "If the end of the - // * output string is reached, *str_len is set to 0" - // *slen = 0; - + if (r <= 0) + return -1; /* wireformat error */ + + written_chars += r; + *d += data_len; + *dlen -= data_len; return written_chars; }