From 9a20db4b5257d56d2d6030a20ad42f5e0dc9f25b Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Mon, 28 Nov 2022 06:05:48 +0300 Subject: fixed a number of crashes due to empty string arguments or uninitialized registers * An empty but valid teco_string_t can contain NULL pointers. More precisely, a state's done_cb() can be invoked with such empty strings in case of empty string arguments. Also a registers get_string() can return the NULL pointer for existing registers with uninitialized string parts. * In all of these cases, the language should treat "uninitialized" strings exactly like empty strings. * Not doing so, resulted in a number of vulnerabilities. * EN$$ crashed if "_" was uninitialized * The ^E@q and ^ENq string building constructs would crash for existing but uninitialized registers q. * ?$ would crash * ESSETILEXER$$ would crash * This is now fixed. Test cases have been added. * I cannot guarantee that I have found all such cases. Generally, it might be wise to change our definitions and make sure that every teco_string_t must have an associated heap object to be valid. All functions returning pointer+length pairs should consequently also never return NULL pointers. --- src/core-commands.c | 1 + src/file-utils.c | 1 + src/glob.c | 6 ++++++ src/help.c | 5 +++-- src/interface-gtk/interface.c | 1 + src/parser.c | 2 +- src/qreg.c | 1 + src/spawn.c | 1 + src/symbols.c | 5 +++-- tests/testsuite.at | 18 ++++++++++++++++-- 10 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/core-commands.c b/src/core-commands.c index bceb644..5f6ec53 100644 --- a/src/core-commands.c +++ b/src/core-commands.c @@ -1418,6 +1418,7 @@ teco_state_changedir_done(teco_machine_main_t *ctx, const teco_string_t *str, GE "Null-character not allowed in filenames"); return NULL; } + g_assert(home.data != NULL); g_free(dir); dir = home.data; diff --git a/src/file-utils.c b/src/file-utils.c index 0e024f1..da55da2 100644 --- a/src/file-utils.c +++ b/src/file-utils.c @@ -207,6 +207,7 @@ teco_file_expand_path(const gchar *path) if (!qreg->vtable->get_string(qreg, &home.data, &home.len, NULL) || teco_string_contains(&home, '\0')) return g_strdup(path); + g_assert(home.data != NULL); return g_build_filename(home.data, path+1, NULL); } diff --git a/src/glob.c b/src/glob.c index e67df7b..6aea02f 100644 --- a/src/glob.c +++ b/src/glob.c @@ -46,6 +46,9 @@ TECO_DECLARE_STATE(teco_state_glob_filename); void teco_globber_init(teco_globber_t *ctx, const gchar *pattern, GFileTest test) { + if (!pattern) + pattern = ""; + memset(ctx, 0, sizeof(*ctx)); ctx->test = test; @@ -114,6 +117,9 @@ teco_globber_clear(teco_globber_t *ctx) gchar * teco_globber_escape_pattern(const gchar *pattern) { + if (!pattern) + return g_strdup(""); + gsize escaped_len = 1; gchar *escaped, *pout; diff --git a/src/help.c b/src/help.c index 48364de..9f589e1 100644 --- a/src/help.c +++ b/src/help.c @@ -283,10 +283,11 @@ teco_state_help_done(teco_machine_main_t *ctx, const teco_string_t *str, GError "Help topic must not contain null-byte"); return NULL; } - teco_help_topic_t *topic = teco_help_find(str->data); + const gchar *topic_name = str->data ? : ""; + teco_help_topic_t *topic = teco_help_find(topic_name); if (!topic) { g_set_error(error, TECO_ERROR, TECO_ERROR_FAILED, - "Topic \"%s\" not found", str->data); + "Topic \"%s\" not found", topic_name); return NULL; } diff --git a/src/interface-gtk/interface.c b/src/interface-gtk/interface.c index f0c5e52..dc7367a 100644 --- a/src/interface-gtk/interface.c +++ b/src/interface-gtk/interface.c @@ -957,6 +957,7 @@ teco_interface_event_loop(GError **error) "Null-character not allowed in filenames"); return FALSE; } + g_assert(scitecoconfig.data != NULL); #ifdef G_OS_WIN32 /* diff --git a/src/parser.c b/src/parser.c index c25ed0d..575c066 100644 --- a/src/parser.c +++ b/src/parser.c @@ -657,7 +657,7 @@ teco_state_stringbuilding_ctle_quote_input(teco_machine_stringbuilding_t *ctx, g table != &teco_qreg_table_globals); return NULL; } - g_autofree gchar *str_quoted = g_shell_quote(str.data); + g_autofree gchar *str_quoted = g_shell_quote(str.data ? : ""); teco_string_append(ctx->result, str_quoted, strlen(str_quoted)); return &teco_state_stringbuilding_start; diff --git a/src/qreg.c b/src/qreg.c index e07bf9e..ce545cc 100644 --- a/src/qreg.c +++ b/src/qreg.c @@ -490,6 +490,7 @@ teco_qreg_workingdir_set_string(teco_qreg_t *qreg, const gchar *str, gsize len, "Directory contains null-character"); return FALSE; } + g_assert(dir.data != NULL); int ret = g_chdir(dir.data); if (ret) { diff --git a/src/spawn.c b/src/spawn.c index 92b8436..59a286f 100644 --- a/src/spawn.c +++ b/src/spawn.c @@ -236,6 +236,7 @@ teco_state_execute_done(teco_machine_main_t *ctx, const teco_string_t *str, GErr "Command line must not contain null-bytes"); goto gerror; } + g_assert(str->data != NULL); argv = teco_parse_shell_command_line(str->data, error); if (!argv) diff --git a/src/symbols.c b/src/symbols.c index 015b933..ac67662 100644 --- a/src/symbols.c +++ b/src/symbols.c @@ -326,10 +326,11 @@ teco_state_scintilla_lparam_done(teco_machine_main_t *ctx, const teco_string_t * return NULL; } - lParam = (sptr_t)CreateLexer(str->data); + const gchar *lexer = str->data ? : ""; + lParam = (sptr_t)CreateLexer(lexer); if (!lParam) { g_set_error(error, TECO_ERROR, TECO_ERROR_FAILED, - "Lexilla lexer \"%s\" not found.", str->data); + "Lexilla lexer \"%s\" not found.", lexer); return NULL; } } else if (str->len > 0) { diff --git a/tests/testsuite.at b/tests/testsuite.at index 2e48973..dd37053 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -93,8 +93,22 @@ AT_SETUP([Q-Register stack cleanup]) AT_CHECK([$SCITECO -e '@<:@a'], 0, ignore, ignore) AT_CLEANUP -AT_SETUP([Empty search]) -AT_CHECK([$SCITECO -e '@S//'], 0, ignore, ignore) +AT_SETUP([Uninitialized "_"-register]) +AT_CHECK([$SCITECO -e ":@S//\"S(0/0)'"], 0, ignore, ignore) +AT_CHECK([$SCITECO -e ":@EN///\"S(0/0)'"], 0, ignore, ignore) +AT_CLEANUP + +AT_SETUP([Uninitialized Q-Register in string building]) +AT_CHECK([$SCITECO -e '@I/^E@a/'], 0, ignore, ignore) +AT_CHECK([$SCITECO -e '@I/^ENa/'], 0, ignore, ignore) +AT_CLEANUP + +AT_SETUP([Empty help topic]) +AT_CHECK([$SCITECO -e '@?//'], 1, ignore, ignore) +AT_CLEANUP + +AT_SETUP([Empty lexer name]) +AT_CHECK([$SCITECO -e '@ES/SETILEXER//'], 1, ignore, ignore) AT_CLEANUP AT_BANNER([Known Bugs]) -- cgit v1.2.3