Skip to content

Commit fa05dcf

Browse files
committed
Fix bug with libcurl timeouts.
It's possible that libcurl disables a timer by requesting timeout_ms=-1. It doesn't seem to expect IO to be processed at this time and I managed to trigger a once-off valgrind crash with a double-free as a result.
1 parent a292a57 commit fa05dcf

File tree

2 files changed

+7
-7
lines changed

2 files changed

+7
-7
lines changed

src/dns_poller.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ static void ares_cb(void *arg, int status, int __attribute__((unused)) timeouts,
9595
static ev_tstamp get_timeout(dns_poller_t *d)
9696
{
9797
static struct timeval max_tv = {.tv_sec = 5, .tv_usec = 0};
98-
struct timeval tv, *tvp = ares_timeout(d->ares, &max_tv, &tv);
98+
struct timeval tv;
99+
struct timeval *tvp = ares_timeout(d->ares, &max_tv, &tv);
99100
ev_tstamp after = tvp->tv_sec + tvp->tv_usec * 1e-6;
100101
return after ? after : 0.1;
101102
}

src/https_client.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
#include <ctype.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
12
#include <errno.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
23
#include <ev.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
34
#include <math.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
45
#include <netinet/in.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
6+
#include <stdio.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
57
#include <string.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
68
#include <sys/socket.h> // NOLINT(llvmlibc-restrict-system-libc-headers)
7-
#include <ctype.h>
89

910
#include "https_client.h"
1011
#include "logging.h"
@@ -203,7 +204,7 @@ static int https_fetch_ctx_process_response(https_client_t *client,
203204
ELOG("CURLINFO_CONTENT_TYPE: %s", curl_easy_strerror(res));
204205
} else {
205206
if (str_resp == NULL ||
206-
strncmp(str_resp, DOH_CONTENT_TYPE, sizeof(DOH_CONTENT_TYPE) - 1)) { // at least, start with it
207+
strncmp(str_resp, DOH_CONTENT_TYPE, sizeof(DOH_CONTENT_TYPE) - 1) != 0) { // at least, start with it
207208
ELOG("Invalid response Content-Type: %s", str_resp ? str_resp : "UNSET");
208209
faulty_response = 1;
209210
}
@@ -216,7 +217,7 @@ static int https_fetch_ctx_process_response(https_client_t *client,
216217
ELOG("CURLINFO_REDIRECT_URL: %s", curl_easy_strerror(res));
217218
} else if (str_resp != NULL) {
218219
ELOG("Request would be redirected to: %s", str_resp);
219-
if (strcmp(str_resp, client->opt->resolver_url)) {
220+
if (strcmp(str_resp, client->opt->resolver_url) != 0) {
220221
ELOG("Please update Resolver URL to avoid redirection!");
221222
}
222223
}
@@ -421,12 +422,10 @@ static int multi_timer_cb(CURLM __attribute__((unused)) *multi,
421422
long timeout_ms, void *userp) {
422423
https_client_t *c = (https_client_t *)userp;
423424
ev_timer_stop(c->loop, &c->timer);
424-
if (timeout_ms > 0) {
425+
if (timeout_ms >= 0) {
425426
// NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
426427
ev_timer_init(&c->timer, timer_cb, timeout_ms / 1000.0, 0);
427428
ev_timer_start(c->loop, &c->timer);
428-
} else {
429-
timer_cb(c->loop, &c->timer, 0);
430429
}
431430
return 0;
432431
}

0 commit comments

Comments
 (0)