doh: cleanups in ECH related functions

- make local_decode_rdata_name use dynbuf instead of calloc + memcpy
- avoid extra memdup in local_decode_rdata_alpn
- no need to if() before free()
- use memdup instead of calloc + memcpy in Curl_doh_decode_httpsrr

Reviewed-by: Stephen Farrell
Closes #13526
This commit is contained in:
Daniel Stenberg 2024-05-03 15:06:54 +02:00
parent 3a082cd3c5
commit 0a94d18241
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2

View File

@ -1045,50 +1045,45 @@ static CURLcode local_decode_rdata_name(unsigned char **buf, size_t *remaining,
{ {
unsigned char *cp = NULL; unsigned char *cp = NULL;
int rem = 0; int rem = 0;
char *thename = NULL, *tp = NULL;
unsigned char clen = 0; /* chunk len */ unsigned char clen = 0; /* chunk len */
struct dynbuf thename;
DEBUGASSERT(buf && remaining && dnsname);
if(!buf || !remaining || !dnsname) if(!buf || !remaining || !dnsname)
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
rem = (int)*remaining; rem = (int)*remaining;
thename = calloc(1, CURL_MAXLEN_host_name); if(rem <= 0) {
if(!thename) Curl_dyn_free(&thename);
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
}
Curl_dyn_init(&thename, CURL_MAXLEN_host_name);
cp = *buf; cp = *buf;
tp = thename;
clen = *cp++; clen = *cp++;
if(clen == 0) { if(clen == 0) {
/* special case - return "." as name */ /* special case - return "." as name */
thename[0] = '.'; if(Curl_dyn_addn(&thename, ".", 1))
thename[1] = 0x00; return CURLE_OUT_OF_MEMORY;
} }
while(clen) { while(clen) {
if(clen >= rem) { if(clen >= rem) {
free(thename); Curl_dyn_free(&thename);
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
} }
if(((tp - thename) + clen) > CURL_MAXLEN_host_name) { if(Curl_dyn_addn(&thename, cp, clen) ||
free(thename); Curl_dyn_addn(&thename, ".", 1))
return CURLE_OUT_OF_MEMORY; return CURLE_TOO_LARGE;
}
memcpy(tp, cp, clen);
tp += clen;
*tp++ = '.';
cp += clen; cp += clen;
rem -= (clen + 1); rem -= (clen + 1);
if(rem <= 0) { if(rem <= 0) {
free(thename); Curl_dyn_free(&thename);
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
} }
clen = *cp++; clen = *cp++;
} }
*buf = cp; *buf = cp;
if(rem <= 0) {
free(thename);
return CURLE_OUT_OF_MEMORY;
}
*remaining = rem - 1; *remaining = rem - 1;
*dnsname = thename; *dnsname = Curl_dyn_ptr(&thename);
return CURLE_OK; return CURLE_OK;
} }
@ -1108,7 +1103,7 @@ static CURLcode local_decode_rdata_alpn(unsigned char *rrval, size_t len,
*/ */
int remaining = (int) len; int remaining = (int) len;
char *oval; char *oval;
size_t olen = 0, i; size_t i;
unsigned char *cp = rrval; unsigned char *cp = rrval;
struct dynbuf dval; struct dynbuf dval;
@ -1137,13 +1132,10 @@ static CURLcode local_decode_rdata_alpn(unsigned char *rrval, size_t len,
} }
remaining -= (int)tlen; remaining -= (int)tlen;
} }
olen = Curl_dyn_len(&dval); /* this string is always null terminated */
/* I think the + 1 here is ok but it could trigger a read error */ oval = Curl_dyn_ptr(&dval);
oval = (char *)Curl_memdup(Curl_dyn_ptr(&dval), olen + 1);
if(!oval) if(!oval)
goto err; goto err;
Curl_dyn_free(&dval);
oval[olen]='\0';
*alpns = oval; *alpns = oval;
return CURLE_OK; return CURLE_OK;
err: err:
@ -1192,11 +1184,10 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len,
lhrr = calloc(1, sizeof(struct Curl_https_rrinfo)); lhrr = calloc(1, sizeof(struct Curl_https_rrinfo));
if(!lhrr) if(!lhrr)
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
lhrr->val = calloc(1, len); lhrr->val = Curl_memdup(rrval, len);
if(!lhrr->val) if(!lhrr->val)
goto err; goto err;
lhrr->len = len; lhrr->len = len;
memcpy(lhrr->val, rrval, len);
if(remaining <= 2) if(remaining <= 2)
goto err; goto err;
lhrr->priority = (uint16_t)((cp[0] << 8) + cp[1]); lhrr->priority = (uint16_t)((cp[0] << 8) + cp[1]);
@ -1245,11 +1236,8 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len,
return CURLE_OK; return CURLE_OK;
err: err:
if(lhrr) { if(lhrr) {
if(lhrr->target)
free(lhrr->target); free(lhrr->target);
if(lhrr->echconfiglist)
free(lhrr->echconfiglist); free(lhrr->echconfiglist);
if(lhrr->val)
free(lhrr->val); free(lhrr->val);
free(lhrr); free(lhrr);
} }