From f7df66d116905bba152ace14a44189f82c45d2ac Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Mon, 11 Jan 2021 17:58:14 -0500 Subject: [PATCH 01/13] Fix display issue when reporting an incorrect argument for an optional arg An optional argument must be preceeded by the name of the optional argument. If the user entered an invalid argument, instead of showing the value the user entered, the 'name' of the argument was repeated. For instance: command argname InvalidValue would display: Problem parsing command setting argname with value argname instead of: Problme parsing command setting argname with value InvalidValue --- libcli.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libcli.c b/libcli.c index cca6249..d677a60 100644 --- a/libcli.c +++ b/libcli.c @@ -941,8 +941,8 @@ void cli_get_completions(struct cli_def *cli, const char *command, char lastchar // Special case for getting help with no defined optargs.... comphelp->num_entries = -1; } - if (stage->status) { - // if we had an error here we need to redraw the commandline + if (stage->status) { + // if we had an error here we need to redraw the commandline cli_reprompt(cli); } } @@ -3218,7 +3218,7 @@ static void cli_get_optarg_comphelp(struct cli_def *cli, struct cli_optarg *opta static void cli_int_parse_optargs(struct cli_def *cli, struct cli_pipeline_stage *stage, struct cli_command *cmd, char lastchar, struct cli_comphelp *comphelp) { struct cli_optarg *optarg = NULL, *oaptr = NULL; - int word_idx, word_incr, candidate_idx; + int word_idx, value_idx, word_incr, candidate_idx; struct cli_optarg *candidates[CLI_MAX_LINE_WORDS]; char *value; int num_candidates = 0; @@ -3351,6 +3351,7 @@ static void cli_int_parse_optargs(struct cli_def *cli, struct cli_pipeline_stage // Set some values for use later - makes code much easier to read value = stage->words[word_idx]; + value_idx = word_idx; oaptr = candidates[0]; validator = oaptr->validator; if ((oaptr->flags & (CLI_CMD_OPTIONAL_FLAG | CLI_CMD_ARGUMENT) && word_idx == (stage->num_words - 1)) || @@ -3368,6 +3369,7 @@ static void cli_int_parse_optargs(struct cli_def *cli, struct cli_pipeline_stage goto done; } value = stage->words[word_idx + 1]; + value_idx = word_idx + 1; } /* @@ -3405,7 +3407,7 @@ static void cli_int_parse_optargs(struct cli_def *cli, struct cli_pipeline_stage } } else { cli_error(cli, "%sProblem parsing command setting %s with value %s", lastchar == '\0' ? "" : "\n", oaptr->name, - stage->words[word_idx]); + stage->words[value_idx]); cli_reprompt(cli); stage->error_word = stage->words[word_idx]; stage->status = CLI_ERROR; From 13f5bb23b2bb8914844a45f60d281aab8b7eef4b Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Thu, 14 Jan 2021 17:12:38 -0500 Subject: [PATCH 02/13] Multiple fixes for help formatting, display of parsing errors, make/spec file --- Makefile | 4 ++-- clitest.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ libcli.c | 19 +++++++++++++++---- libcli.h | 3 ++- libcli.spec | 21 ++++++++++++++++++++- 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index e90e144..95e455e 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ PREFIX = /usr/local MAJOR = 1 MINOR = 10 -REVISION = 4 +REVISION = 5 LIB = libcli.so LIB_STATIC = libcli.a @@ -79,7 +79,7 @@ install: $(TARGET_LIBS) rpmprep: rm -rf libcli-$(MAJOR).$(MINOR).$(REVISION) mkdir libcli-$(MAJOR).$(MINOR).$(REVISION) - cp -R libcli.{c,h} libcli.spec clitest.c Makefile COPYING README.md doc libcli-$(MAJOR).$(MINOR).$(REVISION) + cp -R libcli.c libcli.h libcli.spec clitest.c Makefile COPYING README.md doc libcli-$(MAJOR).$(MINOR).$(REVISION) tar zcvf libcli-$(MAJOR).$(MINOR).$(REVISION).tar.gz --exclude CVS --exclude *.tar.gz libcli-$(MAJOR).$(MINOR).$(REVISION) rm -rf libcli-$(MAJOR).$(MINOR).$(REVISION) diff --git a/clitest.c b/clitest.c index 466cf67..0632b66 100644 --- a/clitest.c +++ b/clitest.c @@ -329,6 +329,25 @@ int check1_validator(struct cli_def *cli, UNUSED(const char *name), UNUSED(const return CLI_OK; } +int cmd_deep_dive(struct cli_def *cli, const char *command, char *argv[], int argc) { + cli_print(cli, "Raw commandline was <%s>", cli->pipeline->cmdline); + return CLI_OK; +} + +int int_validator(struct cli_def *cli, const char *name, const char *value) { + // Verify 'value' is a positive number + long len; + char *endptr; + int rc = CLI_OK; + + printf("int_validator called\n"); + errno = 0; + len = strtol(value, &endptr, 10); + if ((endptr == value) || (*endptr != '\0') || ((errno == ERANGE) && ((len == LONG_MIN) || (len == LONG_MAX)))) + return CLI_ERROR; + return rc; +} + int cmd_string(struct cli_def *cli, const char *command, char *argv[], int argc) { int i; cli_print(cli, "Raw commandline was <%s>", cli->pipeline->cmdline); @@ -340,6 +359,17 @@ int cmd_string(struct cli_def *cli, const char *command, char *argv[], int argc) } return CLI_OK; } +int cmd_long_name(struct cli_def *cli, const char *command, char *argv[], int argc) { + int i; + cli_print(cli, "Raw commandline was <%s>", cli->pipeline->cmdline); + cli_print(cli, "Value for text argument is <%s>", cli_get_optarg_value(cli, "text", NULL)); + + cli_print(cli, "Found %d 'extra' arguments after 'text' argument was processed", argc); + for (i = 0; i != argc; i++) { + cli_print(cli, " Extra arg %d = <%s>", i + 1, argv[i]); + } + return CLI_OK; +} void run_child(int x) { struct cli_command *c; @@ -448,6 +478,22 @@ void run_child(int x) { cli_register_command(cli, NULL, "context", cmd_context, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, "Test a user-specified context"); + struct cli_command *d1, *d2, *d3; + + d1 = cli_register_command(cli, NULL, "deep", NULL, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, "top level deep dive cmd"); + d2 = cli_register_command(cli, d1, "dive", NULL, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, "mid level dep dive cmd"); + d3 = cli_register_command(cli, d2, "cmd", cmd_deep_dive, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, + "bottom level dep dive cmd"); + o = cli_register_optarg(d3, "howdeep", CLI_CMD_ARGUMENT, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, "Specify how deep", NULL, + int_validator, NULL); + o = cli_register_optarg(d3, "howlong", CLI_CMD_OPTIONAL_ARGUMENT, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, + "Specify how long", NULL, int_validator, NULL); + + c = cli_register_command( + cli, NULL, "serioously_long_cammand_to_test_with", cmd_long_name, PRIVILEGE_UNPRIVILEGED, MODE_EXEC, + "show long command name with " + "newline\nand_a_really_long_line_that_is_much_longer_than_80_columns_to_show_that_wrap_case"); + cli_set_auth_callback(cli, check_auth); cli_set_enable_callback(cli, check_enable); // Test reading from a file diff --git a/libcli.c b/libcli.c index d677a60..387764f 100644 --- a/libcli.c +++ b/libcli.c @@ -2803,9 +2803,10 @@ static int cli_int_locate_command(struct cli_def *cli, struct cli_command *comma if (c->callback) { rc = CLI_OK; goto CORRECT_CHECKS; - } else { - cli_error(cli, "Invalid %s \"%s\"", commands->parent ? "argument" : "command", stage->words[start_word]); } + // show the command from word 0 up until the 'bad' word at start_word+1 + cli_error(cli, "Invalid command \"%s %s\"", cli_command_name(cli, c), stage->words[start_word + 1]); + return CLI_ERROR; } return rc; } @@ -2821,6 +2822,7 @@ static int cli_int_locate_command(struct cli_def *cli, struct cli_command *comma stage->command = c; stage->first_unmatched = start_word + 1; stage->first_optarg = stage->first_unmatched; + // cli_int_parse_optargs will display any detected errors... cli_int_parse_optargs(cli, stage, c, '\0', NULL); rc = stage->status; } @@ -2844,8 +2846,8 @@ static int cli_int_locate_command(struct cli_def *cli, struct cli_command *comma goto AGAIN; } - if (start_word == 0) - cli_error(cli, "Invalid %s \"%s\"", commands->parent ? "argument" : "command", stage->words[start_word]); + // display this if we matched against absolutely nothing.... + if (start_word == 0) cli_error(cli, "Invalid command \"%s\"", stage->words[start_word]); return CLI_ERROR_ARG; } @@ -3053,6 +3055,13 @@ void cli_int_wrap_help_line(char *nameptr, char *helpptr, struct cli_comphelp *c */ do { + // note - 22 is used because name is always indented 2 spaces + if ((nameptr != emptystring) && (namewidth > 22)) { + if (asprintf(&line, "%s", nameptr) < 0) break; + cli_add_comphelp_entry(comphelp, line); + nameptr = emptystring; + namewidth = 22; + } toprint = strlen(helpptr); if (toprint > availwidth) { toprint = availwidth; @@ -3076,6 +3085,8 @@ void cli_int_wrap_help_line(char *nameptr, char *helpptr, struct cli_comphelp *c nameptr = emptystring; helpptr += toprint; + // regardless of how long the command is, indent by 20 chars on all following lines + namewidth = 22; // advance to first non whitespace while (helpptr && isspace(*helpptr)) helpptr++; } while (*helpptr); diff --git a/libcli.h b/libcli.h index 020ea63..0e05249 100644 --- a/libcli.h +++ b/libcli.h @@ -13,7 +13,7 @@ extern "C" { #define LIBCLI_VERSION_MAJOR 1 #define LIBCLI_VERISON_MINOR 10 -#define LIBCLI_VERISON_REVISION 3 +#define LIBCLI_VERISON_REVISION 5 #define LIBCLI_VERSION ((LIBCLI_VERSION_MAJOR << 16) | (LIBCLI_VERSION_MINOR << 8) | LIBCLI_VERSION_REVISION) #define CLI_OK 0 @@ -29,6 +29,7 @@ extern "C" { #define CLI_BUILDMODE_EXTEND -10 #define CLI_BUILDMODE_CANCEL -11 #define CLI_BUILDMODE_EXIT -12 +#define CLI_INCOMPLETE_COMMAND -13 #define MAX_HISTORY 256 diff --git a/libcli.spec b/libcli.spec index fa2bc94..7e23e8b 100644 --- a/libcli.spec +++ b/libcli.spec @@ -1,4 +1,4 @@ -Version: 1.10.4 +Version: 1.10.5 Summary: Cisco-like telnet command-line library Name: libcli Release: 1 @@ -67,6 +67,25 @@ rm -rf $RPM_BUILD_ROOT %defattr(-, root, root) %changelog +* Thu Jan 14 2021 Rob Sanders 1.10.5 +- Fix issue where the help for 'long command name' winds up running into + the actual help text w/o any spaces. Now a long command will be on a line + by itself with the line having the indented help text. +- Change error notification of command processing to make it clearer where + an error was found when processing a command, rather than displaying + all of the command elements from the offending word back to the top + level command. +- Fix incorrect internal version defined in libcli.h for the 1.10.4 release +- Minor change in makefile to create rpm from source code +- Minor additions to clitest to show 'fixed' behavior above. + +* Mon Jan 11 2021 Rob Sanders 1.10.5 +- Fix display issue when optional argument fails validation, so it will show + the invalid *value* rather than repeat the name + +* Sun Jun 7 2020 Danial Zaoui +- Fix function prototype in libcli.h to avoid error when compiled with 'strict-prototypes' + * Tue Mar 3 2020 Rob Sanders 1.10.4-1 - Fix segfault issue found during tab/help processing - Minor fix of version on previous changelog record From 79bd14119ebf3c0eb60174cbb95a815374770591 Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 19:04:08 -0500 Subject: [PATCH 03/13] Fix unused variable in routine --- libcli.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libcli.c b/libcli.c index 387764f..8405168 100644 --- a/libcli.c +++ b/libcli.c @@ -2908,7 +2908,6 @@ void cli_int_free_pipeline(struct cli_pipeline *pipeline) { for (i = 0; i < pipeline->num_words; i++) free_z(pipeline->words[i]); free_z(pipeline->cmdline); free_z(pipeline); - pipeline = NULL; } void cli_int_show_pipeline(struct cli_def *cli, struct cli_pipeline *pipeline) { From 5d14ef8322ffcf30e95a27434f54a22c12629d0f Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 19:04:51 -0500 Subject: [PATCH 04/13] Fix missing check after strdup, ensure error return if NULL, fix error message --- libcli.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libcli.c b/libcli.c index 8405168..55120ed 100644 --- a/libcli.c +++ b/libcli.c @@ -2465,6 +2465,10 @@ int cli_int_execute_buildmode(struct cli_def *cli) { char *value = NULL; cmdline = strdup(cli_command_name(cli, cli->buildmode->command)); + if (!cmdline) { + cli_error(cli, "Unable to allocate memory to process buildmode commandline"); + rc = CLI_ERROR; + } for (optarg = cli->buildmode->command->optargs; rc == CLI_OK && optarg; optarg = optarg->next) { value = NULL; do { @@ -2480,16 +2484,16 @@ int cli_int_execute_buildmode(struct cli_def *cli) { } else if (value) { if (optarg->flags & (CLI_CMD_OPTIONAL_FLAG | CLI_CMD_ARGUMENT)) { if (!(cmdline = cli_int_buildmode_extend_cmdline(cmdline, value))) { - cli_error(cli, "Unable to append to building commandlne"); + cli_error(cli, "Unable to allocate memory to process buildmode commandline"); rc = CLI_ERROR; } } else { if (!(cmdline = cli_int_buildmode_extend_cmdline(cmdline, optarg->name))) { - cli_error(cli, "Unable to append to building commandlne"); + cli_error(cli, "Unable to allocate memory to process buildmode commandline"); rc = CLI_ERROR; } if (!(cmdline = cli_int_buildmode_extend_cmdline(cmdline, value))) { - cli_error(cli, "Unable to append to building commandlne"); + cli_error(cli, "Unable to allocate memory to process buildmode commandline"); rc = CLI_ERROR; } } From 408a2eb292a2fe8db9564bb281b69c2edf34e4ba Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 19:05:28 -0500 Subject: [PATCH 05/13] Fix unneeded strdup/free of search pattern text --- libcli.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libcli.c b/libcli.c index 55120ed..ac0b2d8 100644 --- a/libcli.c +++ b/libcli.c @@ -1889,9 +1889,9 @@ int cli_match_filter_init(struct cli_def *cli, int argc, char **argv, struct cli if (!state) return CLI_ERROR; if (!strcmp(cli->pipeline->current_stage->words[0], "include")) { - state->match.string = strdup(search_pattern); + state->match.string = search_pattern; } else if (!strcmp(cli->pipeline->current_stage->words[0], "exclude")) { - state->match.string = strdup(search_pattern); + state->match.string = search_pattern; state->flags = MATCH_INVERT; #ifndef WIN32 } else { @@ -1942,8 +1942,6 @@ int cli_match_filter(UNUSED(struct cli_def *cli), const char *string, void *data if (!string) { if (state->flags & MATCH_REGEX) regfree(&state->match.re); - else - free(state->match.string); free(state); return CLI_OK; From f18d9b231ae7d2105c10c47dab049d21d7e6cd5d Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 20:22:24 -0500 Subject: [PATCH 06/13] Fix additional places strdup/free used for filters, several places no check of return value being done --- libcli.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/libcli.c b/libcli.c index ac0b2d8..acec159 100644 --- a/libcli.c +++ b/libcli.c @@ -820,6 +820,7 @@ static char *join_words(int argc, char **argv) { } p = malloc(len + 1); + if (!p) return NULL; p[0] = 0; for (i = 0; i < argc; i++) { @@ -1940,8 +1941,7 @@ int cli_match_filter(UNUSED(struct cli_def *cli), const char *string, void *data int r = CLI_ERROR; if (!string) { - if (state->flags & MATCH_REGEX) - regfree(&state->match.re); + if (state->flags & MATCH_REGEX) regfree(&state->match.re); free(state); return CLI_OK; @@ -1971,10 +1971,13 @@ struct cli_range_filter_state { int cli_range_filter_init(struct cli_def *cli, int argc, char **argv, struct cli_filter *filt) { struct cli_range_filter_state *state; - char *from = strdup(cli_get_optarg_value(cli, "range_start", NULL)); - char *to = strdup(cli_get_optarg_value(cli, "range_end", NULL)); + char *from = cli_get_optarg_value(cli, "range_start", NULL); + char *to = cli_get_optarg_value(cli, "range_end", NULL); // Do not have to check from/to since we would not have gotten here if we were missing a required argument. + // Note that since those pointers are not NULL, we don't have to strdup them or free them - just use the pointer + // from the command line processing and continue + filt->filter = cli_range_filter; filt->data = state = calloc(sizeof(struct cli_range_filter_state), 1); if (state) { @@ -1982,8 +1985,6 @@ int cli_range_filter_init(struct cli_def *cli, int argc, char **argv, struct cli state->to = to; return CLI_OK; } else { - free_z(from); - free_z(to); return CLI_ERROR; } } @@ -1993,8 +1994,6 @@ int cli_range_filter(UNUSED(struct cli_def *cli), const char *string, void *data int r = CLI_ERROR; if (!string) { - free_z(state->from); - free_z(state->to); free_z(state); return CLI_OK; } @@ -2465,7 +2464,7 @@ int cli_int_execute_buildmode(struct cli_def *cli) { cmdline = strdup(cli_command_name(cli, cli->buildmode->command)); if (!cmdline) { cli_error(cli, "Unable to allocate memory to process buildmode commandline"); - rc = CLI_ERROR; + rc = CLI_ERROR; } for (optarg = cli->buildmode->command->optargs; rc == CLI_OK && optarg; optarg = optarg->next) { value = NULL; @@ -3402,6 +3401,13 @@ static void cli_int_parse_optargs(struct cli_def *cli, struct cli_pipeline_stage if (oaptr->flags & CLI_CMD_REMAINDER_OF_LINE) { char *combined = NULL; combined = join_words(stage->num_words - word_idx, stage->words + word_idx); + if (!combined) { + cli_error(cli, "%sUnable to allocate memory for command processing", lastchar == '\0' ? "" : "\n"); + cli_reprompt(cli); + stage->error_word = stage->words[word_idx]; + stage->status = CLI_ERROR; + goto done; + } set_value_return = cli_set_optarg_value(cli, oaptr->name, combined, 0); free_z(combined); } else { From 3344c6d0db9238511183dc9dd6b52f8cf6f92ad4 Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 20:23:47 -0500 Subject: [PATCH 07/13] Fix pass_matches to check return from crypt() call --- libcli.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libcli.c b/libcli.c index acec159..8b929d7 100644 --- a/libcli.c +++ b/libcli.c @@ -988,6 +988,7 @@ void cli_regular_interval(struct cli_def *cli, int seconds) { #define DES_PREFIX "{crypt}" // To distinguish clear text from DES crypted #define MD5_PREFIX "$1$" +// returns 0 on fail/error, 1 if password checks out static int pass_matches(const char *pass, const char *attempt) { int des; if ((des = !strncasecmp(pass, DES_PREFIX, sizeof(DES_PREFIX) - 1))) pass += sizeof(DES_PREFIX) - 1; @@ -996,7 +997,10 @@ static int pass_matches(const char *pass, const char *attempt) { // TODO(dparrish): Find a small crypt(3) function for use on windows if (des || !strncmp(pass, MD5_PREFIX, sizeof(MD5_PREFIX) - 1)) attempt = crypt(attempt, pass); #endif - + if (!attempt) { + // silent return here... + return 0; + } return !strcmp(pass, attempt); } From 09059a959ebe6a86d576386f7c0afd65682eeaa5 Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 20:25:14 -0500 Subject: [PATCH 08/13] Fix cli_init() to check status when adding default commands and correctly cleanup if a problem --- libcli.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/libcli.c b/libcli.c index 8b929d7..275e882 100644 --- a/libcli.c +++ b/libcli.c @@ -585,7 +585,7 @@ struct cli_def *cli_init() { cli->buf_size = 1024; if (!(cli->buffer = calloc(cli->buf_size, 1))) { - free_z(cli); + cli_done(cli); return 0; } cli->telnet_protocol = 1; @@ -600,17 +600,29 @@ struct cli_def *cli_init() { cli_register_command(cli, 0, "disable", cli_disable, PRIVILEGE_PRIVILEGED, MODE_EXEC, "Turn off privileged commands"); c = cli_register_command(cli, 0, "configure", 0, PRIVILEGE_PRIVILEGED, MODE_EXEC, "Enter configuration mode"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_command(cli, c, "terminal", cli_int_configure_terminal, PRIVILEGE_PRIVILEGED, MODE_EXEC, "Conlfigure from the terminal"); // And now the built in filters c = cli_register_filter(cli, "begin", cli_range_filter_init, cli_range_filter, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Begin with lines that match"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_optarg(c, "range_start", CLI_CMD_ARGUMENT | CLI_CMD_REMAINDER_OF_LINE, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Begin showing lines that match", NULL, NULL, NULL); c = cli_register_filter(cli, "between", cli_range_filter_init, cli_range_filter, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Between lines that match"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_optarg(c, "range_start", CLI_CMD_ARGUMENT, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Begin showing lines that match", NULL, NULL, NULL); cli_register_optarg(c, "range_end", CLI_CMD_ARGUMENT | CLI_CMD_REMAINDER_OF_LINE, PRIVILEGE_UNPRIVILEGED, MODE_ANY, @@ -621,16 +633,28 @@ struct cli_def *cli_init() { c = cli_register_filter(cli, "exclude", cli_match_filter_init, cli_match_filter, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Exclude lines that match"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_optarg(c, "search_pattern", CLI_CMD_ARGUMENT | CLI_CMD_REMAINDER_OF_LINE, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Search pattern", NULL, NULL, NULL); c = cli_register_filter(cli, "include", cli_match_filter_init, cli_match_filter, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Include lines that match"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_optarg(c, "search_pattern", CLI_CMD_ARGUMENT | CLI_CMD_REMAINDER_OF_LINE, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Search pattern", NULL, NULL, NULL); c = cli_register_filter(cli, "grep", cli_match_filter_init, cli_match_filter, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Include lines that match regex (options: -v, -i, -e)"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_optarg(c, "search_flags", CLI_CMD_HYPHENATED_OPTION, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Search flags (-[ivx]", NULL, cli_search_flags_validator, NULL); cli_register_optarg(c, "search_pattern", CLI_CMD_ARGUMENT | CLI_CMD_REMAINDER_OF_LINE, PRIVILEGE_UNPRIVILEGED, @@ -638,6 +662,10 @@ struct cli_def *cli_init() { c = cli_register_filter(cli, "egrep", cli_match_filter_init, cli_match_filter, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Include lines that match extended regex"); + if (!c) { + cli_done(cli); + return 0; + } cli_register_optarg(c, "search_flags", CLI_CMD_HYPHENATED_OPTION, PRIVILEGE_UNPRIVILEGED, MODE_ANY, "Search flags (-[ivx]", NULL, cli_search_flags_validator, NULL); cli_register_optarg(c, "search_pattern", CLI_CMD_ARGUMENT | CLI_CMD_REMAINDER_OF_LINE, PRIVILEGE_UNPRIVILEGED, From 8ecddd51e03ae2b11e8e6191c83c4face4ce5333 Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Fri, 22 Jan 2021 20:26:31 -0500 Subject: [PATCH 09/13] Fix buildmode to check when adding 'new' commands, don't need to verify cli valid Note - check of cli!=NULL is ok, but with check in comples if statement it caused status code analysis tool to complain --- libcli.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/libcli.c b/libcli.c index 275e882..6ad2c66 100644 --- a/libcli.c +++ b/libcli.c @@ -2362,11 +2362,12 @@ void cli_int_free_buildmode(struct cli_def *cli) { int cli_int_enter_buildmode(struct cli_def *cli, struct cli_pipeline_stage *stage, char *mode_text) { struct cli_optarg *optarg; struct cli_command *c; + struct cli_optarg *o; struct cli_buildmode *buildmode; struct cli_optarg *buildmodeOptarg = NULL; int rc = CLI_BUILDMODE_START; - if (!cli || !(buildmode = (struct cli_buildmode *)calloc(1, sizeof(struct cli_buildmode)))) { + if (!(buildmode = (struct cli_buildmode *)calloc(1, sizeof(struct cli_buildmode)))) { cli_error(cli, "Unable to build buildmode mode for command %s", stage->command->command); rc = CLI_BUILDMODE_ERROR; goto out; @@ -2434,16 +2435,36 @@ int cli_int_enter_buildmode(struct cli_def *cli, struct cli_pipeline_stage *stag cli->buildmode->cname = strdup(cli_command_name(cli, stage->command)); // Now add the four 'always there' commands to cancel current mode and to execute the command, show settings, and // unset - cli_int_register_buildmode_command(cli, NULL, "cancel", cli_int_buildmode_cancel_cback, 0, PRIVILEGE_UNPRIVILEGED, + c = cli_int_register_buildmode_command(cli, NULL, "cancel", cli_int_buildmode_cancel_cback, 0, PRIVILEGE_UNPRIVILEGED, cli->mode, "Cancel command"); - cli_int_register_buildmode_command(cli, NULL, "execute", cli_int_buildmode_execute_cback, 0, PRIVILEGE_UNPRIVILEGED, + if (!c) { + rc = CLI_BUILDMODE_ERROR; + goto out; + } + c = cli_int_register_buildmode_command(cli, NULL, "execute", cli_int_buildmode_execute_cback, 0, PRIVILEGE_UNPRIVILEGED, cli->mode, "Execute command"); - cli_int_register_buildmode_command(cli, NULL, "show", cli_int_buildmode_show_cback, 0, PRIVILEGE_UNPRIVILEGED, + if (!c) { + rc = CLI_BUILDMODE_ERROR; + goto out; + } + c = cli_int_register_buildmode_command(cli, NULL, "show", cli_int_buildmode_show_cback, 0, PRIVILEGE_UNPRIVILEGED, cli->mode, "Show current settings"); + if (!c) { + rc = CLI_BUILDMODE_ERROR; + goto out; + } c = cli_int_register_buildmode_command(cli, NULL, "unset", cli_int_buildmode_unset_cback, 0, PRIVILEGE_UNPRIVILEGED, cli->mode, "Unset a setting"); - cli_register_optarg(c, "setting", CLI_CMD_ARGUMENT | CLI_CMD_DO_NOT_RECORD, PRIVILEGE_UNPRIVILEGED, cli->mode, + if (!c) { + rc = CLI_BUILDMODE_ERROR; + goto out; + } + o = cli_register_optarg(c, "setting", CLI_CMD_ARGUMENT | CLI_CMD_DO_NOT_RECORD, PRIVILEGE_UNPRIVILEGED, cli->mode, "setting to clear", cli_int_buildmode_unset_completor, cli_int_buildmode_unset_validator, NULL); + if (!o) { + rc = CLI_BUILDMODE_ERROR; + goto out; + } out: // And lastly set the initial help menu for the unset command From 38bab91d5133b0090559af8e9dc9535e221cd43a Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Wed, 27 Jan 2021 13:49:45 -0500 Subject: [PATCH 10/13] The full name of a command is generated now in 'cli_register_command' rather than on the fly Note that the 'cli->commandname' structure member no longer exists --- libcli.h | 3 ++- libcli.spec | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/libcli.h b/libcli.h index 0e05249..024b2a7 100644 --- a/libcli.h +++ b/libcli.h @@ -71,7 +71,7 @@ struct cli_def { /* internal buffers */ void *conn; void *service; - char *commandname; // temporary buffer for cli_command_name() to prevent leak +// char *commandname; // temporary buffer for cli_command_name() to prevent leak char *buffer; unsigned buf_size; struct timeval timeout_tm; @@ -102,6 +102,7 @@ enum command_types { struct cli_command { char *command; + char *full_command_name; int (*callback)(struct cli_def *, const char *, char **, int); unsigned int unique_len; char *help; diff --git a/libcli.spec b/libcli.spec index 7e23e8b..755e810 100644 --- a/libcli.spec +++ b/libcli.spec @@ -67,6 +67,11 @@ rm -rf $RPM_BUILD_ROOT %defattr(-, root, root) %changelog +* Wed Jan 27 2021 Rob Sa ders 1.10.5 +- Fix possible error where cli_command_name() returns a NULL by + generating full command name when a command is registered. + Note - removed cli->commandname member + * Thu Jan 14 2021 Rob Sanders 1.10.5 - Fix issue where the help for 'long command name' winds up running into the actual help text w/o any spaces. Now a long command will be on a line From 36dfe81ec9895455242a9be0e5606ddc6c273523 Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Wed, 27 Jan 2021 16:51:43 -0500 Subject: [PATCH 11/13] Forgot to include actual source in last commit --- libcli.c | 50 +++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/libcli.c b/libcli.c index 6ad2c66..a405066 100644 --- a/libcli.c +++ b/libcli.c @@ -168,7 +168,7 @@ static int cli_int_validate_pipeline(struct cli_def *cli, struct cli_pipeline *p static int cli_int_execute_pipeline(struct cli_def *cli, struct cli_pipeline *pipeline); inline void cli_int_show_pipeline(struct cli_def *cli, struct cli_pipeline *pipeline); static void cli_int_free_pipeline(struct cli_pipeline *pipeline); -static void cli_register_command_core(struct cli_def *cli, struct cli_command *parent, struct cli_command *c); +static struct cli_command *cli_register_command_core(struct cli_def *cli, struct cli_command *parent, struct cli_command *c); static void cli_int_wrap_help_line(char *nameptr, char *helpptr, struct cli_comphelp *comphelp); static char DELIM_OPT_START[] = "["; @@ -193,15 +193,14 @@ static ssize_t _write(int fd, const void *buf, size_t count) { return written; } -char *cli_command_name(struct cli_def *cli, struct cli_command *command) { +char *cli_int_command_name(struct cli_def *cli, struct cli_command *command) { char *name; char *o; - - if (cli->commandname) { - free(cli->commandname); - cli->commandname = NULL; + + if (command->full_command_name) { + free(command->full_command_name); + command->full_command_name = NULL; } - name = cli->commandname; if (!(name = calloc(1, 1))) return NULL; @@ -215,10 +214,13 @@ char *cli_command_name(struct cli_def *cli, struct cli_command *command) { command = command->parent; free(o); } - cli->commandname = name; return name; } +char *cli_command_name(struct cli_def *cli, struct cli_command *command) { + return command->full_command_name; +} + void cli_set_auth_callback(struct cli_def *cli, int (*auth_callback)(const char *, const char *)) { cli->auth_callback = auth_callback; } @@ -364,13 +366,20 @@ int cli_set_configmode(struct cli_def *cli, int mode, const char *config_desc) { return old; } -void cli_register_command_core(struct cli_def *cli, struct cli_command *parent, struct cli_command *c) { +struct cli_command *cli_register_command_core(struct cli_def *cli, struct cli_command *parent, struct cli_command *c) { struct cli_command *p = NULL; - if (!c) return; + if (!c) return NULL; c->parent = parent; - + + /* Go build the 'full command name' now that told it who its parent is. + * If this fails, clean it up and return a NULL w/o proceeding. + */ + if (!(c->full_command_name = cli_int_command_name(cli, c))) { + cli_free_command(cli, c); + return NULL; + } /* * Figure out we have a chain, or would be the first element on it. * If we'd be the first element, assign as such. @@ -401,7 +410,7 @@ void cli_register_command_core(struct cli_def *cli, struct cli_command *parent, p->next = c; c->previous = p; } - return; + return c; } struct cli_command *cli_register_command(struct cli_def *cli, struct cli_command *parent, const char *command, @@ -426,9 +435,8 @@ struct cli_command *cli_register_command(struct cli_def *cli, struct cli_command free(c); return NULL; } - - cli_register_command_core(cli, parent, c); - return c; + + return cli_register_command_core(cli, parent, c); } static void cli_free_command(struct cli_def *cli, struct cli_command *cmd) { @@ -443,7 +451,7 @@ static void cli_free_command(struct cli_def *cli, struct cli_command *cmd) { free(cmd->command); if (cmd->help) free(cmd->help); if (cmd->optargs) cli_unregister_all_optarg(cmd); - + if (cmd->full_command_name) free(cmd->full_command_name); /* * Ok, update the pointers of anyone who pointed to us. * We have 3 pointers to worry about - parent, previous, and next. @@ -721,7 +729,6 @@ int cli_done(struct cli_def *cli) { if (cli->buildmode) cli_int_free_buildmode(cli); cli_unregister_tree(cli, cli->commands, CLI_ANY_COMMAND); - free_z(cli->commandname); free_z(cli->modestring); free_z(cli->banner); free_z(cli->promptchar); @@ -2125,8 +2132,7 @@ struct cli_command *cli_register_filter(struct cli_def *cli, const char *command } // Filters are all registered at the top level. - cli_register_command_core(cli, NULL, c); - return c; + return cli_register_command_core(cli, NULL, c); } int cli_unregister_filter(struct cli_def *cli, const char *command) { @@ -2353,7 +2359,6 @@ void cli_int_free_buildmode(struct cli_def *cli) { if (!cli || !cli->buildmode) return; cli_unregister_tree(cli, cli->commands, CLI_BUILDMODE_COMMAND); cli->mode = cli->buildmode->mode; - free_z(cli->buildmode->cname); free_z(cli->buildmode->mode_text); cli_int_free_found_optargs(&cli->buildmode->found_optargs); free_z(cli->buildmode); @@ -2432,7 +2437,7 @@ int cli_int_enter_buildmode(struct cli_def *cli, struct cli_pipeline_stage *stag } } } - cli->buildmode->cname = strdup(cli_command_name(cli, stage->command)); + cli->buildmode->cname = cli_command_name(cli, stage->command); // Now add the four 'always there' commands to cancel current mode and to execute the command, show settings, and // unset c = cli_int_register_buildmode_command(cli, NULL, "cancel", cli_int_buildmode_cancel_cback, 0, PRIVILEGE_UNPRIVILEGED, @@ -2504,8 +2509,7 @@ struct cli_command *cli_int_register_buildmode_command(struct cli_def *cli, stru } // Buildmode commmands are all registered at the top level - cli_register_command_core(cli, NULL, c); - return c; + return cli_register_command_core(cli, NULL, c); } int cli_int_execute_buildmode(struct cli_def *cli) { From c8ddd06b42f6d17e70545124eea31d85c87199eb Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Wed, 27 Jan 2021 17:16:13 -0500 Subject: [PATCH 12/13] Merge upstream after accepting pull 63 and 64, update README.md and spec file --- README.md | 11 ++++++++++- libcli.c | 32 ++++++++++++++++++++++---------- libcli.spec | 9 ++++++++- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 4e8b57b..5e8e009 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,13 @@ $ make $ make install ``` +Note - as of version 1.10.5 you have a compile time decision on using select() +or poll() in cli_loop(). The default is to use the legacy 'select()' call. +If built with 'CFLAGS=-DLIBCLI_USE_POLL make' then the poll() system call will +be used instead. One additional check is being made now in cli_loop() to +ensure that the passed file descriptor is in range. If not, an error message +will be sent and cli_loop() will exit with CLI_ERROR. + This will install `libcli.so` into `/usr/local/lib`. If you want to change the location, edit Makefile. @@ -102,7 +109,9 @@ using `cli_set_context(cli, context)` and `cli_get_context(cli)` functions. You are responsible for accepting a TCP connection, and for creating a process or thread to run the cli. Once you are ready to process the connection, call `cli_loop(cli, sock)` to interact with the user on the -given socket. +given socket. Note that as mentioned above, if the select() call is used and +sock is out of range (>= FD_SETSIZE) then cli_loop() will display an error in +both the parent process and to the remote TCP connection before exiting. This function will return when the user exits the cli, either by breaking the connection or entering `quit`. diff --git a/libcli.c b/libcli.c index d5cf264..7ed96a7 100644 --- a/libcli.c +++ b/libcli.c @@ -174,7 +174,8 @@ static int cli_int_validate_pipeline(struct cli_def *cli, struct cli_pipeline *p static int cli_int_execute_pipeline(struct cli_def *cli, struct cli_pipeline *pipeline); inline void cli_int_show_pipeline(struct cli_def *cli, struct cli_pipeline *pipeline); static void cli_int_free_pipeline(struct cli_pipeline *pipeline); -static struct cli_command *cli_register_command_core(struct cli_def *cli, struct cli_command *parent, struct cli_command *c); +static struct cli_command *cli_register_command_core(struct cli_def *cli, struct cli_command *parent, + struct cli_command *c); static void cli_int_wrap_help_line(char *nameptr, char *helpptr, struct cli_comphelp *comphelp); static int cli_socket_wait(int sockfd, struct timeval *tm); @@ -203,7 +204,7 @@ static ssize_t _write(int fd, const void *buf, size_t count) { char *cli_int_command_name(struct cli_def *cli, struct cli_command *command) { char *name; char *o; - + if (command->full_command_name) { free(command->full_command_name); command->full_command_name = NULL; @@ -379,10 +380,10 @@ struct cli_command *cli_register_command_core(struct cli_def *cli, struct cli_co if (!c) return NULL; c->parent = parent; - + /* Go build the 'full command name' now that told it who its parent is. * If this fails, clean it up and return a NULL w/o proceeding. - */ + */ if (!(c->full_command_name = cli_int_command_name(cli, c))) { cli_free_command(cli, c); return NULL; @@ -442,7 +443,7 @@ struct cli_command *cli_register_command(struct cli_def *cli, struct cli_command free(c); return NULL; } - + return cli_register_command_core(cli, parent, c); } @@ -1097,6 +1098,16 @@ int cli_loop(struct cli_def *cli, int sockfd) { } #endif +#ifndef LIBCLI_USE_POLL + /* Do a range check *early*, and punt if we were passed a file descriptor + * that is out of the valid range + */ + if (sockfd >= FD_SETSIZE) { + fprintf(stderr, "CLI_LOOP() called with sockfd > FD_SETSIZE - aborting\n"); + cli_error(cli, "CLI_LOOP() called with sockfd > FD_SETSIZE - exiting cli_loop\n"); + return CLI_ERROR; + } +#endif setbuf(cli->client, NULL); if (cli->banner) cli_error(cli, "%s", cli->banner); @@ -2444,19 +2455,19 @@ int cli_int_enter_buildmode(struct cli_def *cli, struct cli_pipeline_stage *stag // Now add the four 'always there' commands to cancel current mode and to execute the command, show settings, and // unset c = cli_int_register_buildmode_command(cli, NULL, "cancel", cli_int_buildmode_cancel_cback, 0, PRIVILEGE_UNPRIVILEGED, - cli->mode, "Cancel command"); + cli->mode, "Cancel command"); if (!c) { rc = CLI_BUILDMODE_ERROR; goto out; } - c = cli_int_register_buildmode_command(cli, NULL, "execute", cli_int_buildmode_execute_cback, 0, PRIVILEGE_UNPRIVILEGED, - cli->mode, "Execute command"); + c = cli_int_register_buildmode_command(cli, NULL, "execute", cli_int_buildmode_execute_cback, 0, + PRIVILEGE_UNPRIVILEGED, cli->mode, "Execute command"); if (!c) { rc = CLI_BUILDMODE_ERROR; goto out; } c = cli_int_register_buildmode_command(cli, NULL, "show", cli_int_buildmode_show_cback, 0, PRIVILEGE_UNPRIVILEGED, - cli->mode, "Show current settings"); + cli->mode, "Show current settings"); if (!c) { rc = CLI_BUILDMODE_ERROR; goto out; @@ -2468,7 +2479,8 @@ int cli_int_enter_buildmode(struct cli_def *cli, struct cli_pipeline_stage *stag goto out; } o = cli_register_optarg(c, "setting", CLI_CMD_ARGUMENT | CLI_CMD_DO_NOT_RECORD, PRIVILEGE_UNPRIVILEGED, cli->mode, - "setting to clear", cli_int_buildmode_unset_completor, cli_int_buildmode_unset_validator, NULL); + "setting to clear", cli_int_buildmode_unset_completor, cli_int_buildmode_unset_validator, + NULL); if (!o) { rc = CLI_BUILDMODE_ERROR; goto out; diff --git a/libcli.spec b/libcli.spec index 755e810..cbb838b 100644 --- a/libcli.spec +++ b/libcli.spec @@ -67,7 +67,14 @@ rm -rf $RPM_BUILD_ROOT %defattr(-, root, root) %changelog -* Wed Jan 27 2021 Rob Sa ders 1.10.5 +* Wed Jan 27 2021 Gerrit Huizenga 1.10.5 +- Add ppc64le to travis builds + +* Wed Jan 27 2021 Rob Sanders 1.10.5 +- Add additional range chack to cli_loop() if the 'select' call is used, and + punt if sockfd is out of range +- Add preprocessor check (LIBCLI_USE_POLL) to toggle between using poll or + select in cli_loop(). Code submitted on github by @belge-sel. - Fix possible error where cli_command_name() returns a NULL by generating full command name when a command is registered. Note - removed cli->commandname member From 782a38888f5f4bd478a4900d7985581229600051 Mon Sep 17 00:00:00 2001 From: Rob Sanders Date: Wed, 27 Jan 2021 17:21:41 -0500 Subject: [PATCH 13/13] Monir tweak to README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5e8e009..3dde870 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ or poll() in cli_loop(). The default is to use the legacy 'select()' call. If built with 'CFLAGS=-DLIBCLI_USE_POLL make' then the poll() system call will be used instead. One additional check is being made now in cli_loop() to ensure that the passed file descriptor is in range. If not, an error message -will be sent and cli_loop() will exit with CLI_ERROR. +will be sent and the cli_loop() will exit in the child process with CLI_ERROR. This will install `libcli.so` into `/usr/local/lib`. If you want to change the location, edit Makefile. @@ -111,7 +111,7 @@ process or thread to run the cli. Once you are ready to process the connection, call `cli_loop(cli, sock)` to interact with the user on the given socket. Note that as mentioned above, if the select() call is used and sock is out of range (>= FD_SETSIZE) then cli_loop() will display an error in -both the parent process and to the remote TCP connection before exiting. +both the parent process and to the remote TCP connection before exiting that routine. This function will return when the user exits the cli, either by breaking the connection or entering `quit`.