Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various CodeQL complaints #2065

Merged
merged 13 commits into from
Sep 17, 2024
1 change: 0 additions & 1 deletion src/FTL.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include <errno.h>
#include <pthread.h>
#include <sys/prctl.h>
//#include <math.h>
#include <pwd.h>
// syslog
#include <syslog.h>
Expand Down
6 changes: 6 additions & 0 deletions src/api/teleporter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/config/legacy_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/database/sqlite3-ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion src/dnsmasq_interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
2 changes: 1 addition & 1 deletion src/enums.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ enum blocking_status {

enum debug_flag {
DEBUG_NONE = 0,
DEBUG_DATABASE = 1,
DEBUG_DATABASE,
DEBUG_NETWORKING,
DEBUG_LOCKS,
DEBUG_QUERIES,
Expand Down
4 changes: 2 additions & 2 deletions src/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions src/ntp/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}

Expand Down
1 change: 0 additions & 1 deletion src/ntp/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
11 changes: 8 additions & 3 deletions src/overTime.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
Expand Down
17 changes: 7 additions & 10 deletions src/regex.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -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;
Expand All @@ -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:
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/shmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
2 changes: 1 addition & 1 deletion src/timers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
5 changes: 5 additions & 0 deletions src/tools/gravity-parseList.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 3 additions & 4 deletions src/tools/netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/webserver/webserver.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
7 changes: 7 additions & 0 deletions src/webserver/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/zip/gzip.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions src/zip/teleporter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading