Ineffective DbC protections

Dereference before NULL checks.  Thanks to Eric Sesterhenn from X41
D-Sec GmbH for reporting this.

diff --git a/lib/dns/stats.c b/lib/dns/stats.c
index 1833c79f70e6dfad02765b2641dba861339ae6ff..4c430467ece9327141310ede79cd05b1a84394c3 100644 (file)
--- a/lib/dns/stats.c
+++ b/lib/dns/stats.c
@@ -351,11 +351,12 @@ void
 dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
                              dnssecsignstats_type_t operation) {
        uint32_t kval;
-       int num_keys = isc_stats_ncounters(stats->counters) /
-                      dnssecsign_block_size;
 
        REQUIRE(DNS_STATS_VALID(stats) && stats->type == dns_statstype_dnssec);
 
+       int num_keys = isc_stats_ncounters(stats->counters) /
+                      dnssecsign_block_size;
+
        /* Shift algorithm in front of key tag, which is 16 bits */
        kval = (uint32_t)(alg << 16 | id);
 
@@ -398,11 +399,12 @@ dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id, uint8_t alg,
 void
 dns_dnssecsignstats_clear(dns_stats_t *stats, dns_keytag_t id, uint8_t alg) {
        uint32_t kval;
-       int num_keys = isc_stats_ncounters(stats->counters) /
-                      dnssecsign_block_size;
 
        REQUIRE(DNS_STATS_VALID(stats) && stats->type == dns_statstype_dnssec);
 
+       int num_keys = isc_stats_ncounters(stats->counters) /
+                      dnssecsign_block_size;
+
        /* Shift algorithm in front of key tag, which is 16 bits */
        kval = (uint32_t)(alg << 16 | id);
 

diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
index a3b5bd6cb5544d2341d29499075e265b711f8064..d74d02d3e2fd786c362ff4b636c294df8afbedd0 100644 (file)
--- a/lib/isc/netmgr/http.c
+++ b/lib/isc/netmgr/http.c
@@ -1449,7 +1449,7 @@ isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
                   unsigned int timeout) {
        isc_sockaddr_t local_interface;
        isc_nmsocket_t *sock = NULL;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(cb != NULL);
@@ -1457,6 +1457,8 @@ isc_nm_httpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
        REQUIRE(uri != NULL);
        REQUIRE(*uri != '\0');
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                cb(NULL, ISC_R_SHUTTINGDOWN, cbarg);
                return;
@@ -2461,13 +2463,15 @@ isc_nm_listenhttp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
                  isc_nmsocket_t **sockp) {
        isc_nmsocket_t *sock = NULL;
        isc_result_t result;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
 
+       REQUIRE(VALID_NM(mgr));
        REQUIRE(!ISC_LIST_EMPTY(eps->handlers));
        REQUIRE(!ISC_LIST_EMPTY(eps->handler_cbargs));
        REQUIRE(atomic_load(&eps->in_use) == false);
        REQUIRE(isc_tid() == 0);
 
+       worker = &mgr->workers[isc_tid()];
        sock = isc_mem_get(worker->mctx, sizeof(*sock));
        isc__nmsocket_init(sock, worker, isc_nm_httplistener, iface, NULL);
        atomic_init(&sock->h2.max_concurrent_streams,

diff --git a/lib/isc/netmgr/streamdns.c b/lib/isc/netmgr/streamdns.c
index af312506c71b3f5a3a61f77bc041ebff71145e89..a0926f1a3d027ee9686f5f5d3e48242285b3a423 100644 (file)
--- a/lib/isc/netmgr/streamdns.c
+++ b/lib/isc/netmgr/streamdns.c
@@ -372,10 +372,12 @@ isc_nm_streamdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local,
                        unsigned int timeout, isc_tlsctx_t *ctx,
                        isc_tlsctx_client_session_cache_t *client_sess_cache) {
        isc_nmsocket_t *nsock = NULL;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                cb(NULL, ISC_R_SHUTTINGDOWN, cbarg);
                return;
@@ -716,11 +718,13 @@ isc_nm_listenstreamdns(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
                       isc_nmsocket_t **sockp) {
        isc_result_t result;
        isc_nmsocket_t *listener = NULL;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(isc_tid() == 0);
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                return (ISC_R_SHUTTINGDOWN);
        }

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index db07202b80f693d41b553773ac7ee6495eebdd78..548f9bf65033bf9b8c8dca5ad1e6e664b7157a07 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -223,13 +223,15 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
        isc_nmsocket_t *sock = NULL;
        isc__nm_uvreq_t *req = NULL;
        sa_family_t sa_family;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
        uv_os_sock_t fd = -1;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(local != NULL);
        REQUIRE(peer != NULL);
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                connect_cb(NULL, ISC_R_SHUTTINGDOWN, connect_cbarg);
                return;
@@ -446,7 +448,7 @@ isc_nm_listentcp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
        isc_nmsocket_t *sock = NULL;
        uv_os_sock_t fd = -1;
        isc_result_t result = ISC_R_UNSET;
-       isc__networker_t *worker = &mgr->workers[0];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(isc_tid() == 0);
@@ -456,6 +458,7 @@ isc_nm_listentcp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
        }
        REQUIRE(workers <= mgr->nloops);
 
+       worker = &mgr->workers[0];
        sock = isc_mem_get(worker->mctx, sizeof(*sock));
        isc__nmsocket_init(sock, worker, isc_nm_tcplistener, iface, NULL);
 

diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c
index b66afde4055d736ba79c91ade41846c4b85d48f6..d81f25003293a9db1247151ebf97fdd183c7003e 100644 (file)
--- a/lib/isc/netmgr/tlsstream.c
+++ b/lib/isc/netmgr/tlsstream.c
@@ -947,11 +947,13 @@ isc_nm_listentls(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
        isc_result_t result;
        isc_nmsocket_t *tlssock = NULL;
        isc_nmsocket_t *tsock = NULL;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(isc_tid() == 0);
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                return (ISC_R_SHUTTINGDOWN);
        }
@@ -1171,10 +1173,12 @@ isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
                  isc_tlsctx_client_session_cache_t *client_sess_cache,
                  unsigned int timeout) {
        isc_nmsocket_t *sock = NULL;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                connect_cb(NULL, ISC_R_SHUTTINGDOWN, connect_cbarg);
                return;

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 4671b9f9f0a5c0e02b54124acbf94f8b4fea4c08..d9aa3f7716987a4a78d13f1a81e83178c73849da 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -211,11 +211,13 @@ isc_nm_listenudp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface,
        isc_result_t result = ISC_R_UNSET;
        isc_nmsocket_t *sock = NULL;
        uv_os_sock_t fd = -1;
-       isc__networker_t *worker = &mgr->workers[0];
+       isc__networker_t *worker = NULL;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(isc_tid() == 0);
 
+       worker = &mgr->workers[0];
+
        if (isc__nm_closing(worker)) {
                return (ISC_R_SHUTTINGDOWN);
        }
@@ -357,12 +359,14 @@ isc_nm_routeconnect(isc_nm_t *mgr, isc_nm_cb_t cb, void *cbarg) {
        isc_result_t result = ISC_R_SUCCESS;
        isc_nmsocket_t *sock = NULL;
        isc__nm_uvreq_t *req = NULL;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
        uv_os_sock_t fd = -1;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(isc_tid() == 0);
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                return (ISC_R_SHUTTINGDOWN);
        }
@@ -647,7 +651,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
                 isc_nm_cb_t cb, void *cbarg) {
        isc_nmsocket_t *sock = handle->sock;
        const isc_sockaddr_t *peer = &handle->peer;
-       const struct sockaddr *sa = sock->connected ? NULL : &peer->type.sa;
+       const struct sockaddr *sa = NULL;
        isc__nm_uvreq_t *uvreq = NULL;
        isc__networker_t *worker = NULL;
        uint32_t maxudp;
@@ -660,6 +664,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, const isc_region_t *region,
 
        worker = sock->worker;
        maxudp = atomic_load(&worker->netmgr->maxudp);
+       sa = sock->connected ? NULL : &peer->type.sa;
 
        /*
         * We're simulating a firewall blocking UDP packets bigger than
@@ -768,13 +773,15 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
        isc_nmsocket_t *sock = NULL;
        isc__nm_uvreq_t *req = NULL;
        sa_family_t sa_family;
-       isc__networker_t *worker = &mgr->workers[isc_tid()];
+       isc__networker_t *worker = NULL;
        uv_os_sock_t fd = -1;
 
        REQUIRE(VALID_NM(mgr));
        REQUIRE(local != NULL);
        REQUIRE(peer != NULL);
 
+       worker = &mgr->workers[isc_tid()];
+
        if (isc__nm_closing(worker)) {
                cb(NULL, ISC_R_SHUTTINGDOWN, cbarg);
                return;