From 96746a8199b566c4a01ed0fbd5ff09fbf015fc23 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 5 Sep 2019 21:04:51 +0200 Subject: [PATCH] Attach guards to remaining getter functions. Signed-off-by: DL6ER --- src/api/api.c | 52 +++++++++++- src/datastructure.c | 15 ++++ src/dnsmasq_interface.c | 183 +++++++++++++++++++++++++++++++--------- src/gc.c | 22 +++-- src/overTime.c | 10 ++- src/resolve.c | 4 + 6 files changed, 236 insertions(+), 50 deletions(-) diff --git a/src/api/api.c b/src/api/api.c index 3b953776..3b3dc957 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -81,6 +81,9 @@ void getStats(const int *sock) { // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; + if(client->count > 0) activeclients++; } @@ -227,6 +230,8 @@ void getTopDomains(const char *client_message, const int *sock) { // Get domain pointer const domainsData* domain = getDomain(domainID, true); + if(domain == NULL) + continue; temparray[domainID][0] = domainID; if(blocked) @@ -287,6 +292,8 @@ void getTopDomains(const char *client_message, const int *sock) const int domainID = temparray[i][0]; // Get domain pointer const domainsData* domain = getDomain(domainID, true); + if(domain == NULL) + continue; // Skip this domain if there is a filter on it if(excludedomains != NULL && insetupVarsArray(getstr(domain->domainpos))) @@ -397,6 +404,8 @@ void getTopClients(const char *client_message, const int *sock) { // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; temparray[clientID][0] = clientID; // Use either blocked or total count based on request string temparray[clientID][1] = blockedonly ? client->blockedcount : client->count; @@ -435,6 +444,8 @@ void getTopClients(const char *client_message, const int *sock) const int ccount = temparray[i][1]; // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; // Skip this client if there is a filter on it if(excludeclients != NULL && @@ -490,6 +501,8 @@ void getForwardDestinations(const char *client_message, const int *sock) if(sort) { // Get forward pointer const forwardedData* forward = getForward(forwardID, true); + if(forward == NULL) + continue; temparray[forwardID][0] = forwardID; temparray[forwardID][1] = forward->count; @@ -542,6 +555,8 @@ void getForwardDestinations(const char *client_message, const int *sock) // Get forward pointer const forwardedData* forward = getForward(forwardID, true); + if(forward == NULL) + continue; // Get IP and host name of forward destination if available ip = getstr(forward->ippos); @@ -675,6 +690,9 @@ void getAllQueries(const char *client_message, const int *sock) { // Get forward pointer const forwardedData* forward = getForward(i, true); + if(forward == NULL) + continue; + // Try to match the requested string against their IP addresses and // (if available) their host names if(strcmp(getstr(forward->ippos), forwarddest) == 0 || @@ -707,6 +725,8 @@ void getAllQueries(const char *client_message, const int *sock) { // Get domain pointer const domainsData* domain = getDomain(domainID, true); + if(domain == NULL) + continue; // Try to match the requested string if(strcmp(getstr(domain->domainpos), domainname) == 0) @@ -737,6 +757,9 @@ void getAllQueries(const char *client_message, const int *sock) { // Get client pointer const clientsData* client = getClient(i, true); + if(client == NULL) + continue; + // Try to match the requested string if(strcmp(getstr(client->ippos), clientname) == 0 || (client->namepos != 0 && @@ -787,7 +810,8 @@ void getAllQueries(const char *client_message, const int *sock) { const queriesData* query = getQuery(queryID, true); // Check if this query has been create while in maximum privacy mode - if(query->privacylevel >= PRIVACY_MAXIMUM) continue; + if(query == NULL || query->privacylevel >= PRIVACY_MAXIMUM) + continue; // Verify query type if(query->type > TYPE_MAX-1) @@ -844,6 +868,9 @@ void getAllQueries(const char *client_message, const int *sock) const char *clientIPName = NULL; // Get client pointer const clientsData* client = getClient(query->clientID, true); + if(domain == NULL || client == NULL) + continue; + if(strlen(getstr(client->namepos)) > 0) clientIPName = getClientNameString(queryID); else @@ -905,6 +932,8 @@ void getRecentBlocked(const char *client_message, const int *sock) for(int queryID = counters->queries - 1; queryID > 0 ; queryID--) { const queriesData* query = getQuery(queryID, true); + if(query == NULL) + continue; if(query->status == QUERY_GRAVITY || query->status == QUERY_WILDCARD || @@ -915,6 +944,8 @@ void getRecentBlocked(const char *client_message, const int *sock) // Ask subroutine for domain. It may return "hidden" depending on // the privacy settings at the time the query was made const char *domain = getDomainString(queryID); + if(domain == NULL) + continue; if(istelnet[*sock]) ssend(*sock,"%s\n", domain); @@ -1102,6 +1133,8 @@ void getClientsOverTime(const int *sock) { // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; // Check if this client should be skipped if(insetupVarsArray(getstr(client->ippos)) || insetupVarsArray(getstr(client->namepos))) @@ -1125,6 +1158,8 @@ void getClientsOverTime(const int *sock) // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; const int thisclient = client->overTime[slot]; if(istelnet[*sock]) @@ -1166,6 +1201,9 @@ void getClientNames(const int *sock) { // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; + // Check if this client should be skipped if(insetupVarsArray(getstr(client->ippos)) || insetupVarsArray(getstr(client->namepos))) @@ -1181,6 +1219,9 @@ void getClientNames(const int *sock) // Get client pointer const clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; + const char *client_ip = getstr(client->ippos); const char *client_name = getstr(client->namepos); @@ -1207,7 +1248,9 @@ void getUnknownQueries(const int *sock) { const queriesData* query = getQuery(queryID, true); - if(query->status != QUERY_UNKNOWN && query->complete) continue; + if(query == NULL || + (query->status != QUERY_UNKNOWN && query->complete)) + continue; char type[5]; if(query->type == TYPE_A) @@ -1224,6 +1267,9 @@ void getUnknownQueries(const int *sock) // Get client pointer const clientsData* client = getClient(query->clientID, true); + if(domain == NULL || client == NULL) + continue; + // Get client IP string const char *clientIP = getstr(client->ippos); @@ -1261,6 +1307,8 @@ void getDomainDetails(const char *client_message, const int *sock) { // Get domain pointer const domainsData* domain = getDomain(domainID, true); + if(domain == NULL) + continue; if(strcmp(getstr(domain->domainpos), domainString) == 0) { diff --git a/src/datastructure.c b/src/datastructure.c index eafaa92f..e6268953 100644 --- a/src/datastructure.c +++ b/src/datastructure.c @@ -51,6 +51,11 @@ int findForwardID(const char * forwardString, const bool count) // Get forward pointer forwardedData* forward = getForward(forwardID, false); + if(forward == NULL) + { + logg("ERROR: Encountered serious memory error in findForwardID()"); + return -1; + } // Set magic byte forward->magic = MAGICBYTE; @@ -106,6 +111,11 @@ int findDomainID(const char *domainString) // Get domain pointer domainsData* domain = getDomain(domainID, false); + if(domain == NULL) + { + logg("ERROR: Encountered serious memory error in findDomainID()"); + return -1; + } // Set magic byte domain->magic = MAGICBYTE; @@ -162,6 +172,11 @@ int findClientID(const char *clientIP, const bool count) // Get client pointer clientsData* client = getClient(clientID, false); + if(client == NULL) + { + logg("ERROR: Encountered serious memory error in findClientID()"); + return -1; + } // Set magic byte client->magic = MAGICBYTE; diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index ee3493ab..179257bd 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -37,7 +37,7 @@ #include "args.h" static void print_flags(const unsigned int flags); -static void save_reply_type(const unsigned int flags, const int queryID, const struct timeval response); +static void save_reply_type(const unsigned int flags, queriesData* query, const struct timeval response); static unsigned long converttimeval(const struct timeval time) __attribute__((const)); static void block_single_domain_regex(const char *domain); static void detect_blocked_IP(const unsigned short flags, const char* answer, const int queryID); @@ -167,6 +167,17 @@ void _FTL_new_query(const unsigned int flags, const char *name, const struct all // Save everything queriesData* query = getQuery(queryID, false); + if(query == NULL) + { + // Encountered memory error, skip query + // Free allocated memory + free(domainString); + free(clientIP); + // Release thread lock + unlock_shm(); + return; + } + query->magic = MAGICBYTE; query->timestamp = querytimestamp; query->type = querytype; @@ -201,6 +212,16 @@ void _FTL_new_query(const unsigned int flags, const char *name, const struct all // Get client pointer clientsData* client = getClient(clientID, true); + if(client == NULL) + { + // Encountered memory error, skip query + // Free allocated memory + free(domainString); + free(clientIP); + // Release thread lock + unlock_shm(); + return; + } // Update overTime data structure with the new client client->overTime[timeidx]++; @@ -229,13 +250,15 @@ void _FTL_new_query(const unsigned int flags, const char *name, const struct all { // We have to block this domain block_single_domain_regex(domainString); - domain->regexmatch = REGEX_BLOCKED; + if(domain != NULL) + domain->regexmatch = REGEX_BLOCKED; } else { // Explicitly mark as not blocked to skip regex test // next time we see this domain - domain->regexmatch = REGEX_NOTBLOCKED; + if(domain != NULL) + domain->regexmatch = REGEX_NOTBLOCKED; } } @@ -322,7 +345,8 @@ void _FTL_forwarded(const unsigned int flags, const char *name, const struct all // destinations are coming in for the same query) // - the query was formally known as cached but had to be forwarded // (this is a special case further described below) - if(query->complete && query->status != QUERY_CACHE) + // Use short-circuit evaluation to check if query is NULL + if(query == NULL || (query->complete && query->status != QUERY_CACHE)) { free(forward); unlock_shm(); @@ -486,7 +510,8 @@ void _FTL_reply(const unsigned short flags, const char *name, const struct all_a // Check if reply time is still unknown // We only process the first reply in here - if(query->reply != REPLY_UNKNOWN) + // Use short-circuit evaluation to check if query is NULL + if(query == NULL || query->reply != REPLY_UNKNOWN) { // Nothing to be done here unlock_shm(); @@ -498,6 +523,12 @@ void _FTL_reply(const unsigned short flags, const char *name, const struct all_a // Get domain pointer domainsData* domain = getDomain(domainID, true); + if(domain == NULL) + { + // Memory error, skip reply + unlock_shm(); + return; + } // Check if this domain matches exactly const bool isExactMatch = (name != NULL && strcmp(getstr(domain->domainpos), name) == 0); @@ -523,14 +554,16 @@ void _FTL_reply(const unsigned short flags, const char *name, const struct all_a // Update domain blocked counter domain->blockedcount++; - // Get client pointer - clientsData* client = getClient(query->clientID, true); - - // Update client blocked counter - client->blockedcount++; - // Set query status to wildcard query->status = QUERY_WILDCARD; + + // Get client pointer + clientsData* client = getClient(query->clientID, true); + if(client != NULL) + { + // Update client blocked counter + client->blockedcount++; + } } else { @@ -542,7 +575,7 @@ void _FTL_reply(const unsigned short flags, const char *name, const struct all_a } // Save reply type and update individual reply counters - save_reply_type(flags, i, response); + save_reply_type(flags, query, response); // Hereby, this query is now fully determined query->complete = true; @@ -556,7 +589,7 @@ void _FTL_reply(const unsigned short flags, const char *name, const struct all_a query->reply != QUERY_EXTERNAL_BLOCKED_NXRA) { // Save reply type and update individual reply counters - save_reply_type(flags, i, response); + save_reply_type(flags, query, response); // Detect if returned IP indicates that this query was blocked detect_blocked_IP(flags, answer, i); @@ -574,7 +607,7 @@ void _FTL_reply(const unsigned short flags, const char *name, const struct all_a // Hence, isExactMatch is always false // Save reply type and update individual reply counters - save_reply_type(flags, i, response); + save_reply_type(flags, query, response); } else if(isExactMatch && !query->complete) { @@ -621,9 +654,15 @@ static void detect_blocked_IP(const unsigned short flags, const char* answer, co if(config.debug & DEBUG_EXTBLOCKED) { const queriesData* query = getQuery(queryID, true); - const domainsData* domain = getDomain(query->domainID, true); - logg("Upstream responded with known blocking page (IPv4), ID %i:\n\t\"%s\" -> \"%s\"", - queryID, getstr(domain->domainpos), answer); + if(query != NULL) + { + const domainsData* domain = getDomain(query->domainID, true); + if(domain != NULL) + { + logg("Upstream responded with known blocking page (IPv4), ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domain->domainpos), answer); + } + } } // Update status @@ -641,9 +680,15 @@ static void detect_blocked_IP(const unsigned short flags, const char* answer, co if(config.debug & DEBUG_EXTBLOCKED) { const queriesData* query = getQuery(queryID, true); - const domainsData* domain = getDomain(query->domainID, true); - logg("Upstream responded with known blocking page (IPv6), ID %i:\n\t\"%s\" -> \"%s\"", - queryID, getstr(domain->domainpos), answer); + if(query != NULL) + { + const domainsData* domain = getDomain(query->domainID, true); + if(domain != NULL) + { + logg("Upstream responded with known blocking page (IPv6), ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domain->domainpos), answer); + } + } } // Update status @@ -659,9 +704,15 @@ static void detect_blocked_IP(const unsigned short flags, const char* answer, co if(config.debug & DEBUG_EXTBLOCKED) { const queriesData* query = getQuery(queryID, true); - const domainsData* domain = getDomain(query->domainID, true); - logg("Upstream responded with 0.0.0.0, ID %i:\n\t\"%s\" -> \"%s\"", - queryID, getstr(domain->domainpos), answer); + if(query != NULL) + { + const domainsData* domain = getDomain(query->domainID, true); + if(domain != NULL) + { + logg("Upstream responded with 0.0.0.0, ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domain->domainpos), answer); + } + } } // Update status @@ -673,9 +724,15 @@ static void detect_blocked_IP(const unsigned short flags, const char* answer, co if(config.debug & DEBUG_EXTBLOCKED) { const queriesData* query = getQuery(queryID, true); - const domainsData* domain = getDomain(query->domainID, true); - logg("Upstream responded with ::, ID %i:\n\t\"%s\" -> \"%s\"", - queryID, getstr(domain->domainpos), answer); + if(query != NULL) + { + const domainsData* domain = getDomain(query->domainID, true); + if(domain != NULL) + { + logg("Upstream responded with ::, ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domain->domainpos), answer); + } + } } // Update status @@ -687,6 +744,11 @@ static void query_externally_blocked(const int queryID, const unsigned char stat { // Get query pointer queriesData* query = getQuery(queryID, true); + if(query == NULL) + { + // Memory error, skip check for this query + return; + } // Get time index const unsigned int timeidx = query->timeidx; @@ -706,19 +768,23 @@ static void query_externally_blocked(const int queryID, const unsigned char stat // Get forward pointer forwardedData* forward = getForward(query->forwardID, true); - forward->count--; + if(forward != NULL) + forward->count--; } + // Mark query as blocked counters->blocked++; overTime[timeidx].blocked++; // Get domain pointer domainsData* domain = getDomain(query->domainID, true); - domain->blockedcount++; + if(domain != NULL) + domain->blockedcount++; // Get client pointer clientsData* client = getClient(query->clientID, true); - client->blockedcount++; + if(client != NULL) + client->blockedcount++; // Set query status query->status = status; @@ -817,7 +883,8 @@ void _FTL_cache(const unsigned int flags, const char *name, const struct all_add queriesData* query = getQuery(queryID, true); // Skip this query if already marked as complete - if(query->complete) + // Use short-circuit evaluation to check query if query is NULL + if(query == NULL || query->complete) { unlock_shm(); return; @@ -835,6 +902,13 @@ void _FTL_cache(const unsigned int flags, const char *name, const struct all_add // Get client pointer clientsData* client = getClient(query->clientID, true); + if(domain == NULL || client == NULL) + { + // Memory error, skip this cache reply + unlock_shm(); + return; + } + // Mark this query as blocked if domain was matched by a regex if(domain->regexmatch == REGEX_BLOCKED) requesttype = QUERY_WILDCARD; @@ -871,7 +945,7 @@ void _FTL_cache(const unsigned int flags, const char *name, const struct all_add } // Save reply type and update individual reply counters - save_reply_type(flags, queryID, response); + save_reply_type(flags, query, response); // Hereby, this query is now fully determined query->complete = true; @@ -906,6 +980,12 @@ void _FTL_dnssec(const int status, const int id, const char* file, const int lin // Get query pointer queriesData* query = getQuery(queryID, true); + if(query == NULL) + { + // Memory error, skip this DNSSEC details + unlock_shm(); + return; + } // Debug logging if(config.debug & DEBUG_QUERIES) @@ -952,6 +1032,12 @@ void _FTL_upstream_error(const unsigned int rcode, const int id, const char* fil // Get query pointer queriesData* query = getQuery(queryID, true); + if(query == NULL) + { + // Memory error, skip this query + unlock_shm(); + return; + } // Translate dnsmasq's rcode into something we can use const char *rcodestr = NULL; @@ -981,7 +1067,14 @@ void _FTL_upstream_error(const unsigned int rcode, const int id, const char* fil // Get domain pointer const domainsData* domain = getDomain(query->domainID, true); - logg("**** got error report for %s: %s (ID %i, %s:%i)", getstr(domain->domainpos), rcodestr, id, file, line); + // Get domain name + const char *domainname; + if(domain != NULL) + domainname = getstr(domain->domainpos); + else + domainname = ""; + + logg("**** got error report for %s: %s (ID %i, %s:%i)", domainname, rcodestr, id, file, line); if(query->reply == REPLY_OTHER) { @@ -1028,13 +1121,27 @@ void _FTL_header_analysis(const unsigned char header4, const unsigned int rcode, // Get query pointer queriesData* query = getQuery(queryID, true); + if(query == NULL) + { + // Memory error, skip this query + unlock_shm(); + return; + } // Possible debugging information if(config.debug & DEBUG_QUERIES) { // Get domain pointer const domainsData* domain = getDomain(query->domainID, true); - logg("**** %s externally blocked (ID %i, FTL %i, %s:%i)", getstr(domain->domainpos), id, queryID, file, line); + + // Get domain name + const char *domainname; + if(domain != NULL) + domainname = getstr(domain->domainpos); + else + domainname = ""; + + logg("**** %s externally blocked (ID %i, FTL %i, %s:%i)", domainname, id, queryID, file, line); } // Get response time @@ -1045,7 +1152,7 @@ void _FTL_header_analysis(const unsigned char header4, const unsigned int rcode, query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_NXRA); // Store reply type as replied with NXDOMAIN - save_reply_type(F_NEG | F_NXDOMAIN, queryID, response); + save_reply_type(F_NEG | F_NXDOMAIN, query, response); // Unlock shared memory unlock_shm(); @@ -1068,11 +1175,8 @@ void print_flags(const unsigned int flags) free(flagstr); } -void save_reply_type(const unsigned int flags, const int queryID, const struct timeval response) +static void save_reply_type(const unsigned int flags, queriesData* query, const struct timeval response) { - // Get query pointer - queriesData* query = getQuery(queryID, true); - // Iterate through possible values if(flags & F_NEG) { @@ -1256,7 +1360,8 @@ void _FTL_forwarding_failed(const struct server *server, const char* file, const forwardedData* forward = getForward(forwardID, true); // Update counter - forward->failed++; + if(forward != NULL) + forward->failed++; // Clean up and unlock shared memory free(forwarddest); diff --git a/src/gc.c b/src/gc.c index 3e46183c..f7cbbbe5 100644 --- a/src/gc.c +++ b/src/gc.c @@ -67,23 +67,28 @@ void *GC_thread(void *val) for(long int i=0; i < counters->queries; i++) { queriesData* query = getQuery(i, true); + if(query == NULL) + continue; + // Test if this query is too new if(query->timestamp > mintime) break; // Adjust client counter clientsData* client = getClient(query->clientID, true); - client->count--; + if(client != NULL) + client->count--; // Adjust total counters and total over time data const int timeidx = query->timeidx; overTime[timeidx].total--; - // Adjust corresponding overTime counters - client->overTime[timeidx]--; + if(client != NULL) + client->overTime[timeidx]--; // Adjust domain counter (no overTime information) domainsData* domain = getDomain(query->domainID, true); - domain->count--; + if(domain != NULL) + domain->count--; // Get forward pointer forwardedData* forward = getForward(query->forwardID, true); @@ -99,7 +104,8 @@ void *GC_thread(void *val) // Forwarded to an upstream DNS server // Adjust counters counters->forwardedqueries--; - forward->count--; + if(forward != NULL) + forward->count--; overTime[timeidx].forwarded--; break; case QUERY_CACHE: @@ -115,8 +121,10 @@ void *GC_thread(void *val) case QUERY_EXTERNAL_BLOCKED_NULL: // Blocked by upstream provider (fall through) counters->blocked--; overTime[timeidx].blocked--; - domain->blockedcount--; - client->blockedcount--; + if(domain != NULL) + domain->blockedcount--; + if(client != NULL) + client->blockedcount--; break; default: /* That cannot happen */ diff --git a/src/overTime.c b/src/overTime.c index 9449f820..191d1d23 100644 --- a/src/overTime.c +++ b/src/overTime.c @@ -53,8 +53,11 @@ static void initSlot(const unsigned int index, const time_t timestamp) { // Get client pointer clientsData* client = getClient(clientID, true); - // Set overTime data to zero - client->overTime[index] = 0; + if(client != NULL) + { + // Set overTime data to zero + client->overTime[index] = 0; + } } } @@ -160,6 +163,9 @@ void moveOverTimeMemory(const time_t mintime) { // Get query pointer queriesData* query = getQuery(queryID, true); + if(query == NULL) + continue; + // Check if the index would become negative if we adjusted it if(((int)query->timeidx - (int)moveOverTime) < 0) { diff --git a/src/resolve.c b/src/resolve.c index 4bac6875..00e9792f 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -124,6 +124,8 @@ void resolveClients(const bool onlynew) { // Get client pointer clientsData* client = getClient(clientID, true); + if(client == NULL) + continue; // Memory access needs to get locked lock_shm(); @@ -160,6 +162,8 @@ void resolveForwardDestinations(const bool onlynew) { // Get forward pointer forwardedData* forward = getForward(forwardID, true); + if(forward == NULL) + continue; // Memory access needs to get locked lock_shm();