From 6e34b5f472077cc4b1db03559921ce4d8ad6aad5 Mon Sep 17 00:00:00 2001 From: wb2osz Date: Fri, 29 Sep 2017 18:31:19 -0400 Subject: [PATCH] Possible crash when CDIGIPEAT did not have optional alias. --- CHANGES.md | 17 +++++++++++++- cdigipeater.c | 63 +++++++++++++++++++++++++++++++++++++++------------ cdigipeater.h | 13 +++++++---- config.c | 24 ++++++++++++++++---- 4 files changed, 92 insertions(+), 25 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index fad173f..b7e8d93 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,21 @@ # Revision History # +## Version 1.5 -- Development snapshot C -- September 2017 ## + +This is a snapshot of ongoing development towards version of 1.5. Some features might be incomplete or broken or not documented properly. + +### New Features: ### + +- "kissutil" for troubleshooting a KISS TNC or interfacing to an application via files. + + +### Bugs Fixed: ### + +- Possible crash when CDIGIPEAT did not include the optional alias. + + + ## Version 1.5 -- Development snapshot B -- June 2017 ## This is a snapshot of ongoing development towards version of 1.5. Some features might be incomplete or broken or not documented properly. @@ -37,7 +52,7 @@ This is a snapshot of ongoing development towards version of 1.5. Some features ### Bugs Fixed: ### -- Little spelling errors in messages ???? +- Little typographical / spelling errors in messages. ---------- diff --git a/cdigipeater.c b/cdigipeater.c index e531efd..9c40d95 100644 --- a/cdigipeater.c +++ b/cdigipeater.c @@ -1,7 +1,7 @@ // // This file is part of Dire Wolf, an amateur radio packet TNC. // -// Copyright (C) 2016 John Langner, WB2OSZ +// Copyright (C) 2016, 2017 John Langner, WB2OSZ // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -59,7 +59,7 @@ static packet_t cdigipeat_match (int from_chan, packet_t pp, char *mycall_rec, char *mycall_xmit, - regex_t *alias, int to_chan, char *filter_str); + int has_alias, regex_t *alias, int to_chan, char *cfilter_str); /* @@ -149,9 +149,10 @@ void cdigipeater (int from_chan, packet_t pp) packet_t result; result = cdigipeat_match (from_chan, pp, save_audio_config_p->achan[from_chan].mycall, - save_audio_config_p->achan[to_chan].mycall, - &save_cdigi_config_p->alias[from_chan][to_chan], to_chan, - save_cdigi_config_p->filter_str[from_chan][to_chan]); + save_audio_config_p->achan[to_chan].mycall, + save_cdigi_config_p->has_alias[from_chan][to_chan], + &(save_cdigi_config_p->alias[from_chan][to_chan]), to_chan, + save_cdigi_config_p->cfilter_str[from_chan][to_chan]); if (result != NULL) { tq_append (to_chan, TQ_PRIO_0_HI, result); cdigi_count[from_chan][to_chan]++; @@ -171,9 +172,10 @@ void cdigipeater (int from_chan, packet_t pp) packet_t result; result = cdigipeat_match (from_chan, pp, save_audio_config_p->achan[from_chan].mycall, - save_audio_config_p->achan[to_chan].mycall, - &save_cdigi_config_p->alias[from_chan][to_chan], to_chan, - save_cdigi_config_p->filter_str[from_chan][to_chan]); + save_audio_config_p->achan[to_chan].mycall, + save_cdigi_config_p->has_alias[from_chan][to_chan], + &(save_cdigi_config_p->alias[from_chan][to_chan]), to_chan, + save_cdigi_config_p->cfilter_str[from_chan][to_chan]); if (result != NULL) { tq_append (to_chan, TQ_PRIO_0_HI, result); cdigi_count[from_chan][to_chan]++; @@ -203,12 +205,15 @@ void cdigipeater (int from_chan, packet_t pp) * packet is to be transmitted. Could be the same as * mycall_rec or different. * - * alias - Compiled pattern for my station aliases. - * Could be NULL if no aliases. + * has_alias - True if we have an alias. + * + * alias - Optional compiled pattern for my station aliases. + * Do NOT attempt to use this if 'has_alias' is false. * * to_chan - Channel number that we are transmitting to. * - * filter_str - Filter expression string or NULL. + * cfilter_str - Filter expression string for the from/to channel pair or NULL. + * Note that only a subset of the APRS filters are applicable here. * * Returns: Packet object for transmission or NULL. * The original packet is not modified. The caller is responsible for freeing it. @@ -227,20 +232,38 @@ void cdigipeater (int from_chan, packet_t pp) static packet_t cdigipeat_match (int from_chan, packet_t pp, char *mycall_rec, char *mycall_xmit, - regex_t *alias, int to_chan, char *filter_str) + int has_alias, regex_t *alias, int to_chan, char *cfilter_str) { int r; char repeater[AX25_MAX_ADDR_LEN]; int err; char err_msg[100]; +#if DEBUG + text_color_set(DW_COLOR_DEBUG); + dw_printf ("cdigipeat_match (from_chan=%d, pp=%p, mycall_rec=%s, mycall_xmit=%s, has_alias=%d, alias=%p, to_chan=%d, cfilter_str=%s\n", + from_chan, pp, mycall_rec, mycall_xmit, has_alias, alias, to_chan, cfilter_str); +#endif + /* * First check if filtering has been configured. + * Note that we have three different config file filter commands: + * + * FILTER - APRS digipeating and IGate client side. + * Originally this was the only one. + * Should we change it to AFILTER to make it clearer? + * CFILTER - Similar for connected moded digipeater. + * IGFILTER - APRS-IS (IGate) server side - completely diffeent. + * Confusing with similar name but much different idea. + * Maybe this should be renamed to SUBSCRIBE or something like that. + * + * Logically this should come later, after an address/alias match. + * But here we only have to do it once. */ - if (filter_str != NULL) { + if (cfilter_str != NULL) { - if (pfilter(from_chan, to_chan, filter_str, pp, 0) != 1) { + if (pfilter(from_chan, to_chan, cfilter_str, pp, 0) != 1) { return(NULL); } } @@ -286,7 +309,11 @@ static packet_t cdigipeat_match (int from_chan, packet_t pp, char *mycall_rec, c /* * If we have an alias match, substitute MYCALL. */ - if (alias != NULL) { + if (has_alias) { +#if DEBUG + text_color_set(DW_COLOR_DEBUG); + dw_printf ("Checking %s for alias match.\n", repeater); +#endif err = regexec(alias,repeater,0,NULL,0); if (err == 0) { packet_t result; @@ -304,6 +331,12 @@ static packet_t cdigipeat_match (int from_chan, packet_t pp, char *mycall_rec, c dw_printf ("%s\n", err_msg); } } + else { +#if DEBUG + text_color_set(DW_COLOR_DEBUG); + dw_printf ("No alias was specified.\n"); +#endif + } /* * Don't repeat it if we get here. diff --git a/cdigipeater.h b/cdigipeater.h index 5a50b87..69a4b8c 100644 --- a/cdigipeater.h +++ b/cdigipeater.h @@ -23,15 +23,18 @@ struct cdigi_config_s { /* * Rules for each of the [from_chan][to_chan] combinations. */ + int enabled[MAX_CHANS][MAX_CHANS]; // Is it enabled for from/to pair? + int has_alias[MAX_CHANS][MAX_CHANS]; // If there was no alias in the config file, + // the structure below will not be set up + // properly and an attempt to use it could + // result in a crash. (fixed v1.5) + // Not needed for [APRS] DIGIPEAT because + // the alias is mandatory there. regex_t alias[MAX_CHANS][MAX_CHANS]; - int enabled[MAX_CHANS][MAX_CHANS]; - - char *filter_str[MAX_CHANS+1][MAX_CHANS+1]; + char *cfilter_str[MAX_CHANS][MAX_CHANS]; // NULL or optional Packet Filter strings such as "t/m". - // Notice the size of arrays is one larger than normal. - // That extra position is for the IGate. }; /* diff --git a/config.c b/config.c index 4517915..3c1d4e1 100644 --- a/config.c +++ b/config.c @@ -798,9 +798,9 @@ void config_init (char *fname, struct audio_s *p_audio_config, p_audio_config->achan[0].valid = 1; - memset (p_digi_config, 0, sizeof(struct digi_config_s)); + memset (p_digi_config, 0, sizeof(struct digi_config_s)); // APRS digipeater p_digi_config->dedupe_time = DEFAULT_DEDUPE; - memset (p_cdigi_config, 0, sizeof(struct cdigi_config_s)); + memset (p_cdigi_config, 0, sizeof(struct cdigi_config_s)); // Connected mode digipeater memset (p_tt_config, 0, sizeof(struct tt_config_s)); p_tt_config->gateway_enabled = 0; @@ -2276,7 +2276,10 @@ void config_init (char *fname, struct audio_s *p_audio_config, t = split(NULL,0); if (t != NULL) { e = regcomp (&(p_cdigi_config->alias[from_chan][to_chan]), t, REG_EXTENDED|REG_NOSUB); - if (e != 0) { + if (e == 0) { + p_cdigi_config->has_alias[from_chan][to_chan] = 1; + } + else { regerror (e, &(p_cdigi_config->alias[from_chan][to_chan]), message, sizeof(message)); text_color_set(DW_COLOR_ERROR); dw_printf ("Config file: Invalid alias matching pattern on line %d:\n%s\n", @@ -2303,6 +2306,19 @@ void config_init (char *fname, struct audio_s *p_audio_config, * FILTER from-chan to-chan filter_specification_expression * FILTER from-chan IG filter_specification_expression * FILTER IG to-chan filter_specification_expression + * + * + * Note that we have three different config file filter commands: + * + * FILTER - Originally for APRS digipeating but later enhanced + * to include IGate client side. Maybe it should be + * renamed AFILTER to make it clearer after adding CFILTER. + * + * CFILTER - Similar for connected moded digipeater. + * + * IGFILTER - APRS-IS (IGate) server side - completely diffeent. + * I'm not happy with this name because IG sounds like IGate + * which is really the client side. More comments later. */ else if (strcasecmp(t, "FILTER") == 0) { @@ -2442,7 +2458,7 @@ void config_init (char *fname, struct audio_s *p_audio_config, t = " "; /* Empty means permit nothing. */ } - p_cdigi_config->filter_str[from_chan][to_chan] = strdup(t); + p_cdigi_config->cfilter_str[from_chan][to_chan] = strdup(t); //TODO1.2: Do a test run to see errors now instead of waiting.