Skip to content

Commit af02b68

Browse files
committed
Fix four new defects discovered
h2_cb_on_header: Instead of passing value + 6 (unbounded) directly to dohd_url64_check/dohd_url64_decode, the code now copies exactly valuelen - strlen(GETDNS) bytes into a stack-local b64tmp buffer and NUL-terminates it before calling the helpers. h2_cb_on_frame_recv: Added an is_h2_get field to req_slot, set to 1 when a GET :path payload is successfully decoded. The content-type check now reads (!req->content_type_seen && !req->is_h2_get), so valid GET requests aren't rejected with 415. dohd_new_connection: evquick_addevent's return value is now checked; on failure, the wolfSSL object is freed, the socket closed, and the pool slot returned — preventing socket/memory leaks. heap_insert: realloc now uses a temporary pointer _tmp; the original heap->top is only overwritten on success, preventing the pointer loss that left the heap in a corrupted state.
1 parent e26951c commit af02b68

2 files changed

Lines changed: 20 additions & 6 deletions

File tree

src/dohd.c

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ struct req_slot {
281281
uint32_t h2_response_len;
282282
int is_odoh;
283283
int content_type_seen;
284+
int is_h2_get;
284285
odoh_req_ctx odoh_ctx;
285286
uint16_t id;
286287
struct sockaddr *resolver;
@@ -620,6 +621,7 @@ struct req_slot *dns_create_request_h2(struct client_data *cd, uint32_t stream_i
620621
req->timeout_timer = NULL;
621622
req->is_odoh = 0;
622623
req->content_type_seen = 0;
624+
req->is_h2_get = 0;
623625
memset(&req->odoh_ctx, 0, sizeof(req->odoh_ctx));
624626
nghttp2_session_set_stream_user_data(cd->h2_session, stream_id, req);
625627
return req;
@@ -1072,7 +1074,7 @@ static int h2_cb_on_frame_recv(nghttp2_session *session,
10721074
if ((!req)) {
10731075
return 0;
10741076
}
1075-
if (!req->content_type_seen || req->is_odoh < 0 ||
1077+
if ((!req->content_type_seen && !req->is_h2_get) || req->is_odoh < 0 ||
10761078
(oblivion_mode && req->is_odoh == 0) ||
10771079
(!oblivion_mode && req->is_odoh == 1)) {
10781080
nghttp2_nv nva[] = {
@@ -1173,18 +1175,22 @@ static int h2_cb_on_header(nghttp2_session *session,
11731175
if (valuelen > strlen(GETDNS) && (strncmp((char*)value, GETDNS,
11741176
strlen(GETDNS)) == 0) && (valuelen < DNS_BUFFER_MAXSIZE)) {
11751177
uint32_t outlen = DNS_BUFFER_MAXSIZE;
1178+
size_t b64len = valuelen - strlen(GETDNS);
1179+
char b64tmp[DNS_BUFFER_MAXSIZE];
1180+
memcpy(b64tmp, value + strlen(GETDNS), b64len);
1181+
b64tmp[b64len] = '\0';
11761182
req->h2_request_len = 0;
1177-
if(dohd_url64_check((const char*)(value + 6)) == 0) {
1183+
if(dohd_url64_check(b64tmp) == 0) {
11781184
dohd_destroy_request(req);
11791185
return 0;
11801186
}
1181-
outlen = dohd_url64_decode((const char*)(value + 6),
1182-
req->h2_request_buffer);
1187+
outlen = dohd_url64_decode(b64tmp, req->h2_request_buffer);
11831188
if (outlen <= 0) {
11841189
dohd_destroy_request(req);
11851190
return 0;
11861191
}
11871192
req->h2_request_len = outlen;
1193+
req->is_h2_get = 1;
11881194
DOH_Stats.http2_get_requests++;
11891195
check_stats();
11901196
}
@@ -1373,6 +1379,13 @@ static void dohd_new_connection(int __attribute__((unused)) fd,
13731379

13741380
cd->doh_sd = connd;
13751381
cd->ev_doh = evquick_addevent(cd->doh_sd, EVQUICK_EV_READ, tls_read, tls_fail, cd);
1382+
if (!cd->ev_doh) {
1383+
dohprint(DOH_ERR, "ERROR: failed to register client event");
1384+
wolfSSL_free(cd->ssl);
1385+
close(connd);
1386+
mempool_free(client_pool, cd);
1387+
return;
1388+
}
13761389

13771390
/* Insert into hash table - O(1) */
13781391
client_hash_insert(cd);

src/heap.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,13 @@ static inline int heap_insert(struct heap_##type *heap, type *el)
3232
struct heap_element_##type etmp; \
3333
memcpy(&etmp.data, el, sizeof(type)); \
3434
if (++heap->n >= heap->size) { \
35-
heap->top = realloc(heap->top, \
35+
struct heap_element_##type *_tmp = realloc(heap->top, \
3636
(heap->n + 1) * sizeof(struct heap_element_##type)); \
37-
if (!heap->top) { \
37+
if (!_tmp) { \
3838
heap->n--; \
3939
return -1; \
4040
} \
41+
heap->top = _tmp; \
4142
heap->size++; \
4243
} \
4344
etmp.id = heap->last_id++; \

0 commit comments

Comments
 (0)