From 0358c3b861b8cdbfd5eb29b32bd458f6ada016ba Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 23 Jan 2021 08:31:21 +0100 Subject: [PATCH] Test compile regex before adding to the database (we may want to reject it) Signed-off-by: DL6ER --- src/api/list.c | 21 +++++++++- src/database/message-table.c | 3 ++ src/regex.c | 78 +++++++++++++++++++++--------------- src/regex_r.h | 1 + 4 files changed, 68 insertions(+), 35 deletions(-) diff --git a/src/api/list.c b/src/api/list.c index 6892379e..7557ebc1 100644 --- a/src/api/list.c +++ b/src/api/list.c @@ -232,10 +232,18 @@ static int api_list_write(struct ftl_conn *api, else row.oldtype = NULL; + bool okay = true; + char *regex_msg = NULL; + if(listtype == GRAVITY_DOMAINLIST_ALLOW_REGEX || listtype == GRAVITY_DOMAINLIST_DENY_REGEX) + { + // Test validity of this regex + regexData regex = { 0 }; + okay = compile_regex(argument, ®ex, ®ex_msg); + } + // Try to add item to table const char *sql_msg = NULL; - bool okay = false; - if(gravityDB_addToTable(listtype, &row, &sql_msg, api->method)) + if(okay && gravityDB_addToTable(listtype, &row, &sql_msg, api->method)) { if(listtype != GRAVITY_GROUPS) { @@ -275,6 +283,15 @@ static int api_list_write(struct ftl_conn *api, JSON_OBJ_ADD_NULL(json, "sql_msg"); } + // Add regex error (may not be available) + if (regex_msg != NULL) { + JSON_OBJ_COPY_STR(json, "regex_msg", regex_msg); + free(regex_msg); + regex_msg = NULL; + } else { + JSON_OBJ_ADD_NULL(json, "regex_msg"); + } + // Send error reply return send_json_error(api, 400, // 400 Bad Request "database_error", diff --git a/src/database/message-table.c b/src/database/message-table.c index 936813d7..99cd555f 100644 --- a/src/database/message-table.c +++ b/src/database/message-table.c @@ -228,6 +228,9 @@ static bool add_message(enum message_type type, const char *message, void logg_regex_warning(const char *type, const char *warning, const int dbindex, const char *regex) { + if(warning == NULL) + warning = "No further info available"; + // Log to pihole-FTL.log logg("REGEX WARNING: Invalid regex %s filter \"%s\": %s", type, regex, warning); diff --git a/src/regex.c b/src/regex.c index 1ec309c5..9dbfc287 100644 --- a/src/regex.c +++ b/src/regex.c @@ -95,11 +95,8 @@ unsigned int __attribute__((pure)) get_num_regex(const enum regex_type regexid) #define FTL_REGEX_SEP ";" /* Compile regular expressions into data structures that can be used with regexec() to match against a string */ -static bool compile_regex(const char *regexin, const enum regex_type regexid) +bool compile_regex(const char *regexin, regexData *regex, char **message) { - regexData *regex = get_regex_ptr(regexid); - int index = num_regex[regexid]++; - // Extract possible Pi-hole extensions char rgxbuf[strlen(regexin) + 1u]; // Parse special FTL syntax if present @@ -119,46 +116,49 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid) if(sscanf(part, "querytype=%16s", extra)) { // Warn if specified more than one querytype option - if(regex[index].query_type != 0) - logg_regex_warning(regextype[regexid], - "Overwriting previous querytype setting", - regex[index].database_id, regexin); + if(regex->query_type != 0) + { + *message = strdup("Overwriting previous querytype setting"); + return false; + } // Test input string against all implemented query types - for(enum query_types type = TYPE_A; type < TYPE_MAX; type++) + for(enum query_types qtype = TYPE_A; qtype < TYPE_MAX; qtype++) { // Check for querytype - if(strcasecmp(extra, querytypes[type]) == 0) + if(strcasecmp(extra, querytypes[qtype]) == 0) { - regex[index].query_type = type; - regex[index].query_type_inverted = false; + regex->query_type = qtype; + regex->query_type_inverted = false; break; } // Check for INVERTED querytype - else if(extra[0] == '!' && strcasecmp(extra + 1u, querytypes[type]) == 0) + else if(extra[0] == '!' && strcasecmp(extra + 1u, querytypes[qtype]) == 0) { - regex[index].query_type = type; - regex[index].query_type_inverted = true; + regex->query_type = qtype; + regex->query_type_inverted = true; break; } } // Nothing found - if(regex[index].query_type == 0) - logg_regex_warning(regextype[regexid], "Unknown querytype", - regex[index].database_id, regexin); + if(regex->query_type == 0) + { + *message = strdup("Unknown querytype"); + return false; + } // Debug output else if(config.debug & DEBUG_REGEX) { logg(" This regex will %s match query type %s", - regex[index].query_type_inverted ? "NOT" : "ONLY", - querytypes[regex[index].query_type]); + regex->query_type_inverted ? "NOT" : "ONLY", + querytypes[regex->query_type]); } } // option: ";invert" else if(strcasecmp(part, "invert") == 0) { - regex[index].inverted = true; + regex->inverted = true; // Debug output if(config.debug & DEBUG_REGEX) @@ -181,22 +181,20 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid) // We use the extended RegEx flavor (ERE) and specify that matching should // always be case INsensitive - const int errcode = regcomp(®ex[index].regex, rgxbuf, REG_EXTENDED | REG_ICASE | REG_NOSUB); + const int errcode = regcomp(®ex->regex, rgxbuf, REG_EXTENDED | REG_ICASE | REG_NOSUB); if(errcode != 0) { // Get error string and log it - const size_t length = regerror(errcode, ®ex[index].regex, NULL, 0); - char *buffer = calloc(length, sizeof(char)); - (void) regerror (errcode, ®ex[index].regex, buffer, length); - logg_regex_warning(regextype[regexid], buffer, regex[index].database_id, regexin); - free(buffer); - regex[index].available = false; + const size_t length = regerror(errcode, ®ex->regex, NULL, 0); + *message = calloc(length, sizeof(char)); + (void) regerror (errcode, ®ex->regex, *message, length); + regex->available = false; return false; } // Store compiled regex string in buffer - regex[index].string = strdup(regexin); - regex[index].available = true; + regex->string = strdup(regexin); + regex->available = true; return true; } @@ -510,7 +508,16 @@ static void read_regex_table(const enum regex_type regexid) regextype[regexid], num_regex[regexid], rowid, domain); } - compile_regex(domain, regexid); + regexData *this_regex = get_regex_ptr(regexid); + const int index = num_regex[regexid]++; + char *message = NULL; + if(!compile_regex(domain, &this_regex[index], &message) && message != NULL) + { + logg_regex_warning(regextype[regexid], message, + regex->database_id, domain); + free(message); + } + regex[num_regex[regexid]-1].database_id = rowid; // Signal other forks that the regex data has changed and should be updated @@ -614,13 +621,18 @@ int regex_test(const bool debug_mode, const bool quiet, const char *domainin, co { // Compile CLI regex logg("%s Compiling regex filter...", cli_info()); - cli_regex = calloc(1, sizeof(regexData)); + regexData regex = { 0 }; // Compile CLI regex timer_start(REGEX_TIMER); log_ctrl(false, true); // Temporarily re-enable terminal output for error logging - if(!compile_regex(regexin, REGEX_CLI)) + char *message = NULL; + if(!compile_regex(regexin, ®ex, &message) && message != NULL) + { + logg_regex_warning("CLI", message, 0, regexin); + free(message); return EXIT_FAILURE; + } log_ctrl(false, !quiet); // Re-apply quiet option after compilation logg(" Compiled regex filter in %.3f msec\n", timer_elapsed_msec(REGEX_TIMER)); diff --git a/src/regex_r.h b/src/regex_r.h index 45b56303..3f9a7849 100644 --- a/src/regex_r.h +++ b/src/regex_r.h @@ -38,6 +38,7 @@ typedef struct { } regexData; ASSERT_SIZEOF(regexData, 32, 20, 20); +bool compile_regex(const char *regexin, regexData *regex, char **message); unsigned int get_num_regex(const enum regex_type regexid) __attribute__((pure)); int match_regex(const char *input, const DNSCacheData* dns_cache, const int clientID, const enum regex_type regexid, const bool regextest);