upstream/unbound
1
0
Fork 0
mirror of https://github.com/NLnetLabs/unbound.git synced 2025-12-22 07:41:16 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

Apply suggestions from code review

Browse source 
Co-authored-by: Willem Toorop <willem@nlnetlabs.nl>
This commit is contained in:
tcarpay 2021-06-23 10:53:11 +02:00 committed by GitHub
parent 307613ea83
commit eb9891f4ed
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 44 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
sldns/str2wire.c
View file

@ -626,7 +626,7 @@ static int sldns_str2wire_svcparam_key_cmp(const void *a, const void *b)
static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len) static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
{ {
size_t nparams = 0, i, j; size_t nparams = 0, i, j;
uint8_t new_rdata[65536]; uint8_t new_rdata[LDNS_MAX_RDFLEN];
uint8_t* new_rdata_ptr = new_rdata; uint8_t* new_rdata_ptr = new_rdata;
uint8_t* svcparams[64]; uint8_t* svcparams[64];
uint8_t* mandatory = NULL; uint8_t* mandatory = NULL;
@ -650,7 +650,7 @@ static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
rdata_ptr += svcbparam_len; rdata_ptr += svcbparam_len;
nparams += 1; nparams += 1;
if (nparams > sizeof(svcparams)/8) if (nparams > MAX_NUMBER_OF_SVCPARAMS)
return LDNS_WIREPARSE_ERR_SVCB_TOO_MANY_PARAMS; return LDNS_WIREPARSE_ERR_SVCB_TOO_MANY_PARAMS;
} }
@ -684,7 +684,7 @@ static int sldns_str2wire_check_svcbparams(uint8_t* rdata, uint16_t rdata_len)
if (mandatory) { if (mandatory) {
/* Divide by sizeof(uint16_t)*/ /* Divide by sizeof(uint16_t)*/
uint16_t mandatory_len = sldns_read_uint16(mandatory + 2) >> 1; uint16_t mandatory_len = sldns_read_uint16(mandatory + 2) / sizeof(uint16_t);
/* Guaranteed by sldns_str2wire_svcparam_key_value */ /* Guaranteed by sldns_str2wire_svcparam_key_value */
assert(mandatory_len > 0); assert(mandatory_len > 0);
@ -1360,6 +1360,8 @@ sldns_str2wire_svcbparam_ech_value(const char* val, uint8_t* rd, size_t* rd_len)
/* single 0 represents empty buffer */ /* single 0 represents empty buffer */
if(strcmp(val, "0") == 0) { if(strcmp(val, "0") == 0) {
if (*rd_len < 4)
return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL
sldns_write_uint16(rd, SVCB_KEY_ECH); sldns_write_uint16(rd, SVCB_KEY_ECH);
sldns_write_uint16(rd + 2, 0); sldns_write_uint16(rd + 2, 0);
@ -1399,6 +1401,9 @@ sldns_str2wire_svcbparam_parse_next_unescaped_comma(const char *val)
return NULL; return NULL;
} }
/* The source is already properly unescaped, this double unescaping is purely to allow for
* comma's in comma seperated alpn lists.
*/
static size_t static size_t
sldns_str2wire_svcbparam_parse_copy_unescaped(uint8_t *dst, sldns_str2wire_svcbparam_parse_copy_unescaped(uint8_t *dst,
const char *src, size_t len) const char *src, size_t len)
@ -1422,12 +1427,12 @@ static int
sldns_str2wire_svcbparam_alpn_value(const char* val, sldns_str2wire_svcbparam_alpn_value(const char* val,
uint8_t* rd, size_t* rd_len) uint8_t* rd, size_t* rd_len)
{ {
uint8_t unescaped_dst[65536]; uint8_t unescaped_dst[LDNS_MAX_RDFLEN];
uint8_t *dst = unescaped_dst; uint8_t *dst = unescaped_dst;
const char *next_str; const char *next_str;
size_t str_len; size_t str_len;
size_t dst_len; size_t dst_len;
size_t val_len; size_t val_len;
val_len = strlen(val); val_len = strlen(val);
@ -1456,7 +1461,8 @@ sldns_str2wire_svcbparam_alpn_value(const char* val,
val = next_str + 1; val = next_str + 1;
} }
dst_len = dst - unescaped_dst; dst_len = dst - unescaped_dst;
if (*rd_len < 4 + dst_len)
return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL;
sldns_write_uint16(rd, SVCB_KEY_ALPN); sldns_write_uint16(rd, SVCB_KEY_ALPN);
sldns_write_uint16(rd + 2, dst_len); sldns_write_uint16(rd + 2, dst_len);
memcpy(rd + 4, unescaped_dst, dst_len); memcpy(rd + 4, unescaped_dst, dst_len);
@ -1486,6 +1492,8 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len,
case SVCB_KEY_IPV6HINT: case SVCB_KEY_IPV6HINT:
return LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM; return LDNS_WIREPARSE_ERR_SVCB_MISSING_PARAM;
default: default:
if (*rd_len < 4)
return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL;
sldns_write_uint16(rd, svcparamkey); sldns_write_uint16(rd, svcparamkey);
sldns_write_uint16(rd + 2, 0); sldns_write_uint16(rd + 2, 0);
*rd_len = 4; *rd_len = 4;
@ -1512,6 +1520,8 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len,
return sldns_str2wire_svcbparam_alpn_value(val, rd, rd_len); return sldns_str2wire_svcbparam_alpn_value(val, rd, rd_len);
default: default:
str_len = strlen(val); str_len = strlen(val);
if (*rd_len < 4 + str_len)
return LDNS_WIREPARSE_ERR_BUFFER_TOO_SMALL;
sldns_write_uint16(rd, svcparamkey); sldns_write_uint16(rd, svcparamkey);
sldns_write_uint16(rd + 2, str_len); sldns_write_uint16(rd + 2, str_len);
memcpy(rd + 4, val, str_len); memcpy(rd + 4, val, str_len);
@ -1527,7 +1537,7 @@ sldns_str2wire_svcparam_value(const char *key, size_t key_len,
int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_len) int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_len)
{ {
const char* eq_pos; const char* eq_pos;
char unescaped_val[65536]; char unescaped_val[LDNS_MAX_RDFLEN];
char* val_out = unescaped_val; char* val_out = unescaped_val;
const char* val_in; const char* val_in;
@ -1540,11 +1550,14 @@ int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_len)
/* unescape characters and "" blocks */ /* unescape characters and "" blocks */
if (*val_in == '"') { if (*val_in == '"') {
val_in++; val_in++;
while (*val_in != '"' && sldns_parse_char( (uint8_t*) val_out, &val_in)) { while (*val_in != '"'
&& val_out - unescaped_val < sizeof(unescaped_val) - 1
&& sldns_parse_char( (uint8_t*) val_out, &val_in)) {
val_out++; val_out++;
} }
} else { } else {
while ( sldns_parse_char( (uint8_t*) val_out, &val_in)) { while (val_out - unescaped_val < sizeof(unescaped_val) - 1
&& sldns_parse_char( (uint8_t*) val_out, &val_in)) {
val_out++; val_out++;
} }
} }
@ -1561,8 +1574,6 @@ int sldns_str2wire_svcparam_buf(const char* str, uint8_t* rd, size_t* rd_len)
else { else {
return sldns_str2wire_svcparam_value(str, strlen(str), NULL, rd, rd_len); return sldns_str2wire_svcparam_value(str, strlen(str), NULL, rd, rd_len);
} }
return LDNS_WIREPARSE_ERR_OK;
} }
int sldns_str2wire_rdf_buf(const char* str, uint8_t* rd, size_t* len, int sldns_str2wire_rdf_buf(const char* str, uint8_t* rd, size_t* len,

2
sldns/str2wire.h
View file

@ -36,7 +36,7 @@ struct sldns_struct_lookup_table;
#define SVCB_KEY_NO_DEFAULT_ALPN 2 #define SVCB_KEY_NO_DEFAULT_ALPN 2
#define SVCB_KEY_PORT 3 #define SVCB_KEY_PORT 3
#define SVCB_KEY_IPV4HINT 4 #define SVCB_KEY_IPV4HINT 4
#define SVCB_KEY_ECH 5 #define SVCB_KEY_ECH 5
#define SVCB_KEY_IPV6HINT 6 #define SVCB_KEY_IPV6HINT 6
#define SVCPARAMKEY_COUNT 7 #define SVCPARAMKEY_COUNT 7

39
sldns/wire2str.c
View file

@ -1003,19 +1003,20 @@ static int sldns_wire2str_svcparam_ipv4hint2str(char** s,
if ((data_len % LDNS_IP4ADDRLEN) == 0) { if ((data_len % LDNS_IP4ADDRLEN) == 0) {
if (inet_ntop(AF_INET, data, ip_str, sizeof(ip_str)) == NULL) if (inet_ntop(AF_INET, data, ip_str, sizeof(ip_str)) == NULL)
return 0; /* wireformat error, incorrect size or inet family */ return -1; /* wireformat error, incorrect size or inet family */
w += sldns_str_print(s, slen, "=%s", ip_str); w += sldns_str_print(s, slen, "=%s", ip_str);
data += LDNS_IP4ADDRLEN; data += LDNS_IP4ADDRLEN;
while ((data_len -= LDNS_IP4ADDRLEN) > 0) { while ((data_len -= LDNS_IP4ADDRLEN) > 0) {
if (inet_ntop(AF_INET, data, ip_str, sizeof(ip_str)) == NULL) if (inet_ntop(AF_INET, data, ip_str, sizeof(ip_str)) == NULL)
return 0; /* wireformat error, incorrect size or inet family */ return -1; /* wireformat error, incorrect size or inet family */
w += sldns_str_print(s, slen, ",%s", ip_str); w += sldns_str_print(s, slen, ",%s", ip_str);
data += LDNS_IP4ADDRLEN; data += LDNS_IP4ADDRLEN;
} }
} } else
return -1;
return w; return w;
} }
@ -1031,19 +1032,20 @@ static int sldns_wire2str_svcparam_ipv6hint2str(char** s,
if ((data_len % LDNS_IP6ADDRLEN) == 0) { if ((data_len % LDNS_IP6ADDRLEN) == 0) {
if (inet_ntop(AF_INET6, data, ip_str, sizeof(ip_str)) == NULL) if (inet_ntop(AF_INET6, data, ip_str, sizeof(ip_str)) == NULL)
return 0; /* wireformat error, incorrect size or inet family */ return -1; /* wireformat error, incorrect size or inet family */
w += sldns_str_print(s, slen, "=%s", ip_str); w += sldns_str_print(s, slen, "=%s", ip_str);
data += LDNS_IP6ADDRLEN; data += LDNS_IP6ADDRLEN;
while ((data_len -= LDNS_IP6ADDRLEN) > 0) { while ((data_len -= LDNS_IP6ADDRLEN) > 0) {
if (inet_ntop(AF_INET6, data, ip_str, sizeof(ip_str)) == NULL) if (inet_ntop(AF_INET6, data, ip_str, sizeof(ip_str)) == NULL)
return 0; /* wireformat error, incorrect size or inet family */ return -1; /* wireformat error, incorrect size or inet family */
w += sldns_str_print(s, slen, ",%s", ip_str); w += sldns_str_print(s, slen, ",%s", ip_str);
data += LDNS_IP6ADDRLEN; data += LDNS_IP6ADDRLEN;
} }
} } else
return -1;
return w; return w;
} }
@ -1055,8 +1057,8 @@ static int sldns_wire2str_svcparam_mandatory2str(char** s,
assert(data_len > 0); assert(data_len > 0);
// if (data_len % sizeof(uint16_t)) if (data_len % sizeof(uint16_t))
// return 0; // wireformat error, data_len must be multiple of shorts return -1; // wireformat error, data_len must be multiple of shorts
w += sldns_str_print(s, slen, "="); w += sldns_str_print(s, slen, "=");
w += sldns_print_svcparamkey(s, slen, sldns_read_uint16(data)); w += sldns_print_svcparamkey(s, slen, sldns_read_uint16(data));
data += 2; data += 2;
@ -1076,14 +1078,15 @@ static int sldns_wire2str_svcparam_alpn2str(char** s,
uint8_t *dp = (void *)data; uint8_t *dp = (void *)data;
int w = 0; int w = 0;
assert(data_len > 0); /* Guaranteed by rdata_svcparam_to_string */ assert(data_len > 0); /* Guaranteed by sldns_wire2str_svcparam_scan */
w += sldns_str_print(s, slen, "=\""); w += sldns_str_print(s, slen, "=\"");
while (data_len) { while (data_len) {
/* alpn is list of length byte (str_len) followed by a string of that size */
uint8_t i, str_len = *dp++; uint8_t i, str_len = *dp++;
if (str_len > --data_len) if (str_len > --data_len)
return 0; return -1;
for (i = 0; i < str_len; i++) { for (i = 0; i < str_len; i++) {
if (dp[i] == '"' || dp[i] == '\\') if (dp[i] == '"' || dp[i] == '\\')
@ -1113,22 +1116,18 @@ static int sldns_wire2str_svcparam_ech2str(char** s,
int size; int size;
int w = 0; int w = 0;
assert(data_len > 0); /* Guaranteed by rdata_svcparam_to_string */ assert(data_len > 0); /* Guaranteed by sldns_wire2str_svcparam_scan */
w += sldns_str_print(s, slen, "=\""); w += sldns_str_print(s, slen, "=\"");
size = sldns_b64_ntop(data, data_len, *s, *slen); if ((size = sldns_b64_ntop(data, data_len, *s, *slen)) < 0)
return -1;
(*s) += size; (*s) += size;
(*slen) -= size; (*slen) -= size;
w += sldns_str_print(s, slen, "\""); w += sldns_str_print(s, slen, "\"");
// @TODO fix check
// if(size > *slen) {
// buffer_skip(output, size);
// }
return w + size; return w + size;
} }
@ -1162,9 +1161,9 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
case SVCB_KEY_IPV4HINT: case SVCB_KEY_IPV4HINT:
case SVCB_KEY_IPV6HINT: case SVCB_KEY_IPV6HINT:
case SVCB_KEY_MANDATORY: case SVCB_KEY_MANDATORY:
return LDNS_WIREPARSE_ERR_SYNTAX_MISSING_VALUE; return -1;
default: default:
return LDNS_WIREPARSE_ERR_OK; return written_chars;
} }
} }
@ -1205,7 +1204,7 @@ int sldns_wire2str_svcparam_scan(uint8_t** d, size_t* dlen, char** s, size_t* sl
r += sldns_str_print(s, slen, "%c", ch); r += sldns_str_print(s, slen, "%c", ch);
} }
r += sldns_str_print(s, slen, "%c", '"'); r += sldns_str_print(s, slen, "\"");
break; break;
} }
if (r <= 0) if (r <= 0)

3
sldns/wire2str.h
View file

@ -41,9 +41,6 @@ extern struct sldns_struct_lookup_table* sldns_wireparse_errors;
/** tsig errors are the rcodes with extra (higher) values */ /** tsig errors are the rcodes with extra (higher) values */
extern struct sldns_struct_lookup_table* sldns_tsig_errors; extern struct sldns_struct_lookup_table* sldns_tsig_errors;
/* draft-ietf-dnsop-svcb-https-04: 6. Initial SvcParamKeys */
extern const char *svcparamkey_strs[];
/** /**
* Convert wireformat packet to a string representation * Convert wireformat packet to a string representation
* @param data: wireformat packet data (starting at ID bytes). * @param data: wireformat packet data (starting at ID bytes).

3
testcode/readzone.c
View file

@ -72,7 +72,7 @@ int main(int argc, char *const *argv)
s = sldns_fp2wire_rr_buf(in, rr, &rr_len, &dname_len, &state); s = sldns_fp2wire_rr_buf(in, rr, &rr_len, &dname_len, &state);
if (s) { if (s) {
fprintf( stderr, "parse error %d:%d: %s" fprintf( stderr, "parse error %d:%d: %s\n"
, state.lineno, LDNS_WIREPARSE_OFFSET(s) , state.lineno, LDNS_WIREPARSE_OFFSET(s)
, sldns_get_errorstr_parse(s)); , sldns_get_errorstr_parse(s));
break; break;
@ -103,5 +103,6 @@ int main(int argc, char *const *argv)
} }
if (in) if (in)
fclose(in); fclose(in);
free(str);
return !in || s ? EXIT_FAILURE : EXIT_SUCCESS; return !in || s ? EXIT_FAILURE : EXIT_SUCCESS;
} }