Merge pull request #2065 from pi-hole/fix/various

Fix various CodeQL complaints

src/FTL.h

@@ -33,7 +33,6 @@
 #include <errno.h>
 #include <pthread.h>
 #include <sys/prctl.h>
-//#include <math.h>
 #include <pwd.h>
 // syslog
 #include <syslog.h>

src/api/teleporter.c

@@ -782,6 +782,12 @@ static int process_received_tar_gz(struct ftl_conn *api, struct upload_data *dat
 				log_err("Unable to open file \"%s\" for writing: %s", extract_files[i].destination, strerror(errno));
 				continue;
 			}
+
+			// Restrict permissions to owner read/write only
+			if(fchmod(fileno(fp), S_IRUSR | S_IWUSR) != 0)
+				log_warn("Unable to set permissions on file \"%s\": %s", extract_files[i].destination, strerror(errno));
+
+			// Write file to disk
 			if(fwrite(file, fileSize, 1, fp) != 1)
 			{
 				log_err("Unable to write file \"%s\": %s", extract_files[i].destination, strerror(errno));

src/config/legacy_reader.c

@@ -268,7 +268,7 @@ const char *readFTLlegacy(struct config *conf)
 	buffer = parseFTLconf(fp, "DELAY_STARTUP");
 
 	unsigned int unum;
-	if(buffer != NULL && sscanf(buffer, "%u", &unum) && unum > 0 && unum <= 300)
+	if(buffer != NULL && sscanf(buffer, "%u", &unum) == 1 && unum > 0 && unum <= 300)
 		conf->misc.delay_startup.v.ui = unum;
 
 	// BLOCK_ESNI

src/database/sqlite3-ext.h

@@ -8,5 +8,10 @@
 *  This file is copyright under the latest version of the EUPL.
 *  Please see LICENSE file for your rights under this license. */
 
+#ifndef SQLITE3_EXT_H
+#define SQLITE3_EXT_H
+
 // Initialization point for SQLite3 extensions
 extern int sqlite3_pihole_extensions_init(sqlite3 *db, const char **pzErrMsg, const struct sqlite3_api_routines *pApi);
+
+#endif // SQLITE3_EXT_H

src/dnsmasq_interface.c

@@ -1567,7 +1567,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c
 					blockingreason = "blocked upstream with NXRA address";
 					break;
 			}
-			
+
 			// Known as upstream blocked, we return this result
 			// early, skipping all the lengthy tests below
 			log_debug(DEBUG_QUERIES, "%s is known as %s (expires in %lus)",

src/enums.h

@@ -123,7 +123,7 @@ enum blocking_status {
 
 enum debug_flag {
 	DEBUG_NONE = 0,
-	DEBUG_DATABASE = 1,
+	DEBUG_DATABASE,
 	DEBUG_NETWORKING,
 	DEBUG_LOCKS,
 	DEBUG_QUERIES,

src/log.c

@@ -381,7 +381,7 @@ void FTL_log_helper(const unsigned int n, ...)
 	va_list args;
 	char **arg = calloc(n, sizeof(char*));
 	va_start(args, n);
-	for(unsigned char i = 0; i < n; i++)
+	for(unsigned int i = 0; i < n; i++)
 	{
 		const char *argin = va_arg(args, char*);
 		if(argin == NULL)
@@ -410,7 +410,7 @@ void FTL_log_helper(const unsigned int n, ...)
 	}
 
 	// Free allocated memory
-	for(unsigned char i = 0; i < n; i++)
+	for(unsigned int i = 0; i < n; i++)
 		if(arg[i] != NULL)
 			free(arg[i]);
 	free(arg);

src/ntp/client.c

@@ -93,11 +93,12 @@ static void format_NTP_time(char time_str[TIMESTR_SIZE], const uint64_t ntp_time
 	struct timeval client_time;
 	client_time.tv_sec = NTPtoSEC(ntp_time);
 	client_time.tv_usec = NTPtoUSEC(ntp_time);
-	struct tm *client_tm = localtime(&client_time.tv_sec);
+	struct tm client_tm = {0};
+	localtime_r(&client_time.tv_sec, &client_tm);
 	snprintf(time_str, TIMESTR_SIZE, "%04i-%02i-%02i %02i:%02i:%02i.%06li %s",
-	         client_tm->tm_year + 1900, client_tm->tm_mon + 1, client_tm->tm_mday,
-	         client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec,
-	         (long int)client_time.tv_usec, client_tm->tm_zone);
+	         client_tm.tm_year + 1900, client_tm.tm_mon + 1, client_tm.tm_mday,
+	         client_tm.tm_hour, client_tm.tm_min, client_tm.tm_sec,
+	         (long int)client_time.tv_usec, client_tm.tm_zone);
 	time_str[TIMESTR_SIZE - 1] = '\0';
 }
 

src/ntp/server.c

@@ -19,7 +19,6 @@
 #include <signal.h>
 // clock_gettime()
 #include <sys/time.h>
-//#include <sys/types.h>
 #include <sys/wait.h>
 // wait()
 #include <sys/socket.h>

src/overTime.c

@@ -30,7 +30,9 @@ static void initSlot(const unsigned int index, const time_t timestamp)
 	if(config.debug.overtime.v.b)
 	{
 		char timestr[20];
-		strftime(timestr, 20, "%Y-%m-%d %H:%M:%S", localtime(&timestamp));
+		struct tm tm = { 0 };
+		localtime_r(&timestamp, &tm);
+		strftime(timestr, 20, "%Y-%m-%d %H:%M:%S", &tm);
 		log_debug(DEBUG_OVERTIME, "initSlot(%u, %lu): Zeroing overTime slot at %s", index, (unsigned long)timestamp, timestr);
 	}
 
@@ -73,8 +75,11 @@ void initOverTime(void)
 	if(config.debug.overtime.v.b)
 	{
 		char first[20], last[20];
-		strftime(first, 20, "%Y-%m-%d %H:%M:%S", localtime(&oldest));
-		strftime(last, 20, "%Y-%m-%d %H:%M:%S", localtime(&newest));
+		struct tm tm_o = { 0 }, tm_n = { 0 };
+		localtime_r(&oldest, &tm_o);
+		localtime_r(&newest, &tm_n);
+		strftime(first, 20, "%Y-%m-%d %H:%M:%S", &tm_o);
+		strftime(last, 20, "%Y-%m-%d %H:%M:%S", &tm_n);
 		log_debug(DEBUG_OVERTIME, "initOverTime(): Initializing %i slots from %s (%lu) to %s (%lu)",
 		          OVERTIME_SLOTS, first, (unsigned long)oldest, last, (unsigned long)newest);
 	}

src/regex.c

@@ -34,7 +34,7 @@ const char *regextype[REGEX_MAX] = { "deny", "allow", "CLI" };
 
 static regexData *allow_regex = NULL;
 static regexData  *deny_regex = NULL;
-static regexData   *cli_regex = NULL;
+static regexData    cli_regex = { 0 };
 static unsigned int num_regex[REGEX_MAX] = { 0 };
 unsigned int regex_change = 0;
 static char regex_msg[REGEX_MSG_LEN] = { 0 };
@@ -48,7 +48,7 @@ static inline regexData *get_regex_ptr(const enum regex_type regexid)
 		case REGEX_ALLOW:
 			return allow_regex;
 		case REGEX_CLI:
-			return cli_regex;
+			return &cli_regex;
 		case REGEX_MAX: // Fall through
 		default: // This is not possible
 			return NULL;
@@ -57,7 +57,7 @@ static inline regexData *get_regex_ptr(const enum regex_type regexid)
 
 static inline void free_regex_ptr(const enum regex_type regexid)
 {
-	regexData **regex;
+	regexData **regex = NULL;
 	switch (regexid)
 	{
 		case REGEX_DENY:
@@ -67,8 +67,8 @@ static inline void free_regex_ptr(const enum regex_type regexid)
 			regex = &allow_regex;
 			break;
 		case REGEX_CLI:
-			regex = &cli_regex;
-			break;
+			// cannot be freed
+			return;
 		case REGEX_MAX: // Fall through
 		default: // This is not possible
 			return;
@@ -626,8 +626,7 @@ void free_regex(void)
 {
 	// Return early if we don't use any regex filters
 	if(allow_regex == NULL &&
-	    deny_regex == NULL &&
-	     cli_regex == NULL)
+	    deny_regex == NULL)
 	{
 		log_debug(DEBUG_DATABASE, "Not using any regex filters, nothing to free or reset");
 		return;
@@ -895,15 +894,13 @@ int regex_test(const bool debug_mode, const bool quiet, const char *domainin, co
 	{
 		// Compile CLI regex
 		log_info("%s Compiling regex filter...", cli_info());
-		regexData regex = { 0 };
-		cli_regex = &regex;
 		num_regex[REGEX_CLI] = 1;
 
 		// Compile CLI regex
 		timer_start(REGEX_TIMER);
 		log_ctrl(false, true); // Temporarily re-enable terminal output for error logging
 		char *message = NULL;
-		if(!compile_regex(regexin, &regex, &message) && message != NULL)
+		if(!compile_regex(regexin, &cli_regex, &message) && message != NULL)
 		{
 			logg_regex_warning("CLI", message, 0, regexin);
 			free(message);

src/shmem.c

@@ -951,7 +951,7 @@ void reset_per_client_regex(const int clientID)
 void add_per_client_regex(unsigned int clientID)
 {
 	const unsigned int num_regex_tot = get_num_regex(REGEX_MAX); // total number
-	const size_t size = get_optimal_object_size(1, counters->clients * num_regex_tot);
+	const size_t size = get_optimal_object_size(1, (size_t)counters->clients * num_regex_tot);
 	if(size > shm_per_client_regex.size &&
 	   realloc_shm(&shm_per_client_regex, 1, size, true))
 	{

src/timers.c

@@ -16,7 +16,7 @@
 // set_blockingmode()
 #include "config/config.h"
 
-struct timespec t0[NUMTIMERS];
+static struct timespec t0[NUMTIMERS];
 
 void timer_start(const enum timers i)
 {

src/tools/gravity-parseList.h

@@ -8,7 +8,12 @@
 *  This file is copyright under the latest version of the EUPL.
 *  Please see LICENSE file for your rights under this license. */
 
+#ifndef GRAVITY_PARSELIST_H
+#define GRAVITY_PARSELIST_H
+
 #include "FTL.h"
 
 int gravity_parseList(const char *infile, const char *outfile, const char *adlistID, const bool checkOnly, const bool antigravity);
 bool __attribute__((pure)) valid_domain(const char *domain, const size_t len, const bool fqdn_only);
+
+#endif // GRAVITY_PARSELIST_H

src/tools/netlink.c

@@ -1118,17 +1118,16 @@ static int nlquery(const int type, cJSON *json, const bool detailed)
 	memset(&sa, 0, sizeof(sa));
 	sa.nl_family = AF_NETLINK;
 
-	ssize_t len = nlrequest(fd, &sa, type);
-	if(len < 0)
+	if(!nlrequest(fd, &sa, type))
 	{
 		log_info("nlrequest error: %s", strerror(errno));
 		return -1;
 	}
 
-	char buf[BUFLEN];
 	uint32_t nl_msg_type;
 	do {
-		len = nlgetmsg(fd, &sa, buf, BUFLEN);
+		char buf[BUFLEN];
+		ssize_t len = nlgetmsg(fd, &sa, buf, BUFLEN);
 		nl_msg_type = parse_nl_msg(buf, len, json, detailed);
 	} while (nl_msg_type != NLMSG_DONE && nl_msg_type != NLMSG_ERROR);
 

src/webserver/webserver.c

@@ -606,7 +606,7 @@ void FTL_rewrite_pattern(char *filename, unsigned long filename_buf_len)
 	filename_lp = append_to_path(filename, ".lp");
 	if(filename_lp == NULL)
 	{
-		//Failed to allocate memory for filename!");
+		// Failed to allocate memory for filename
 		return;
 	}
 

src/webserver/x509.c

@@ -110,6 +110,10 @@ static bool write_to_file(const char *filename, const char *type, const char *su
 		return false;
 	}
 
+	// Restrict permissions to owner read/write only
+	if(fchmod(fileno(f), S_IRUSR | S_IWUSR) != 0)
+		log_warn("Unable to set permissions on file \"%s\": %s", targetname, strerror(errno));
+
 	// Write key (if provided)
 	if(key != NULL)
 	{
@@ -234,6 +238,9 @@ bool generate_certificate(const char* certfile, bool rsa, const char *domain)
 	char not_after[16] = { 0 };
 	strftime(not_before, sizeof(not_before), "%Y%m%d%H%M%S", tm);
 	tm->tm_year += 30; // 30 years from now
+	// Check for leap year, and adjust the date accordingly
+	const bool isLeapYear = tm->tm_year % 4 == 0 && (tm->tm_year % 100 != 0 || tm->tm_year % 400 == 0);
+	tm->tm_mday = tm->tm_mon == 2 && tm->tm_mday == 29 && !isLeapYear ? 28 : tm->tm_mday;
 	strftime(not_after, sizeof(not_after), "%Y%m%d%H%M%S", tm);
 
 	// 1. Create CA certificate

src/zip/gzip.c

@@ -8,6 +8,7 @@
 *  This file is copyright under the latest version of the EUPL.
 *  Please see LICENSE file for your rights under this license. */
 
+#include "FTL.h"
 #include "gzip.h"
 #include "log.h"
 
@@ -315,6 +316,10 @@ bool inflate_file(const char *infilename, const char *outfilename, bool verbose)
 		return false;
 	}
 
+	// Restrict permissions to owner read/write only
+	if(fchmod(fileno(outfile), S_IRUSR | S_IWUSR) != 0)
+		log_warn("Unable to set permissions on file \"%s\": %s", outfilename, strerror(errno));
+
 	// Get file size
 	fseek(infile, 0, SEEK_END);
 	const long sc = ftell(infile);
@@ -408,6 +413,10 @@ bool deflate_file(const char *infilename, const char *outfilename, bool verbose)
 		return false;
 	}
 
+	// Restrict permissions to owner read/write only
+	if(fchmod(fileno(outfile), S_IRUSR | S_IWUSR) != 0)
+		log_warn("Unable to set permissions on file \"%s\": %s", outfilename, strerror(errno));
+
 	// Get file size
 	fseek(infile, 0, SEEK_END);
 	const long size_uncompressed = ftell(infile);

src/zip/teleporter.c

@@ -816,6 +816,12 @@ bool read_teleporter_zip_from_disk(const char *filename)
 	// Process ZIP archive
 	char hint[ERRBUF_SIZE] = "";
 	cJSON *imported_files = cJSON_CreateArray();
+	if(imported_files == NULL)
+	{
+		log_err("Failed to create JSON array for imported files");
+		free(ptr);
+		return false;
+	}
 	const char *error = read_teleporter_zip(ptr, size, hint, NULL, imported_files);
 
 	if(error != NULL)