diff --git a/src/core/settings/plugins/ifcfg-rh/shvar.c b/src/core/settings/plugins/ifcfg-rh/shvar.c index 371756e8c3..80644b64fd 100644 --- a/src/core/settings/plugins/ifcfg-rh/shvar.c +++ b/src/core/settings/plugins/ifcfg-rh/shvar.c @@ -153,17 +153,42 @@ _escape_ansic(const char *source) const char *p; char * dest; char * q; + gsize n_alloc; nm_assert(source); - p = (const char *) source; - /* Each source byte needs maximally four destination chars (\777) */ - q = dest = g_malloc(strlen(source) * 4 + 1 + 3); + n_alloc = 4; + for (p = source; p[0]; p++) { + switch (*p) { + case '\b': + case '\f': + case '\n': + case '\r': + case '\t': + case '\v': + case '\\': + case '"': + case '\'': + n_alloc += 2; + break; + default: + if ((*p < ' ') || (*p >= 0177)) + n_alloc += 4; + else + n_alloc += 1; + break; + } + } + + dest = g_malloc(n_alloc); + + q = dest; *q++ = '$'; *q++ = '\''; - while (*p) { + for (p = source; p[0]; p++) { + nm_assert(q < &dest[n_alloc]); switch (*p) { case '\b': *q++ = '\\'; @@ -205,12 +230,11 @@ _escape_ansic(const char *source) *q++ = *p; break; } - p++; } *q++ = '\''; *q++ = '\0'; - nm_assert(q - dest <= strlen(source) * 4 + 1 + 3); + nm_assert(q - dest == n_alloc); return dest; } @@ -218,7 +242,7 @@ _escape_ansic(const char *source) /*****************************************************************************/ #define _char_req_escape(ch) NM_IN_SET(ch, '"', '\\', '$', '`') -#define _char_req_escape_old(ch) NM_IN_SET(ch, '"', '\\', '\'', '$', '`', '~') +#define _char_req_escape_old(ch) NM_IN_SET(ch, '"', '\\', '$', '`', '\'', '~') #define _char_req_quotes(ch) NM_IN_SET(ch, ' ', '\'', '~', '\t', '|', '&', ';', '(', ')', '<', '>') const char * @@ -227,8 +251,10 @@ svEscape(const char *s, char **to_free) char *new; gsize mangle = 0; gboolean requires_quotes = FALSE; - int newlen; - size_t i, j, slen; + gsize n_alloc; + gsize slen; + gsize i; + gsize j; for (slen = 0; s[slen]; slen++) { if (_char_req_escape(s[slen])) @@ -248,8 +274,8 @@ svEscape(const char *s, char **to_free) return s; } - newlen = slen + mangle + 3; /* 3 is extra ""\0 */ - new = g_malloc(newlen); + n_alloc = slen + mangle + 3; /* 3 is extra ""\0 */ + new = g_malloc(n_alloc); j = 0; new[j++] = '"'; @@ -261,7 +287,7 @@ svEscape(const char *s, char **to_free) new[j++] = '"'; new[j++] = '\0'; - nm_assert(j == slen + mangle + 3); + nm_assert(j == n_alloc); *to_free = new; return new; diff --git a/src/libnm-client-impl/nm-vpn-plugin-old.c b/src/libnm-client-impl/nm-vpn-plugin-old.c index be4e1cf292..f60bf19c7b 100644 --- a/src/libnm-client-impl/nm-vpn-plugin-old.c +++ b/src/libnm-client-impl/nm-vpn-plugin-old.c @@ -420,12 +420,12 @@ _connect_generic(NMVpnPluginOld * plugin, GVariant * properties, GVariant * details) { - NMVpnPluginOldPrivate *priv = NM_VPN_PLUGIN_OLD_GET_PRIVATE(plugin); - NMVpnPluginOldClass * vpn_class = NM_VPN_PLUGIN_OLD_GET_CLASS(plugin); - NMConnection * connection; - gboolean success = FALSE; - GError * error = NULL; - guint fail_stop_timeout = 0; + NMVpnPluginOldPrivate *priv = NM_VPN_PLUGIN_OLD_GET_PRIVATE(plugin); + NMVpnPluginOldClass * vpn_class = NM_VPN_PLUGIN_OLD_GET_CLASS(plugin); + gs_unref_object NMConnection *connection = NULL; + gboolean success = FALSE; + gs_free_error GError *error = NULL; + guint fail_stop_timeout = 0; if (priv->state != NM_VPN_SERVICE_STATE_STOPPED && priv->state != NM_VPN_SERVICE_STATE_INIT) { g_dbus_method_invocation_return_error(context, @@ -444,7 +444,7 @@ _connect_generic(NMVpnPluginOld * plugin, NM_VPN_PLUGIN_ERROR_BAD_ARGUMENTS, "Invalid connection: %s", error->message); - g_clear_error(&error); + return; } priv->interactive = FALSE; @@ -478,15 +478,13 @@ _connect_generic(NMVpnPluginOld * plugin, /* Add a timer to make sure we do not wait indefinitely for the successful connect. */ connect_timer_start(plugin); } else { - g_dbus_method_invocation_take_error(context, error); + g_dbus_method_invocation_take_error(context, g_steal_pointer(&error)); /* Stop the plugin from an idle handler so that the Connect * method return gets sent before the STOP StateChanged signal. */ schedule_fail_stop(plugin, fail_stop_timeout); } - - g_object_unref(connection); } static void diff --git a/src/libnm-client-impl/nm-vpn-service-plugin.c b/src/libnm-client-impl/nm-vpn-service-plugin.c index 016c761c40..18a85e913b 100644 --- a/src/libnm-client-impl/nm-vpn-service-plugin.c +++ b/src/libnm-client-impl/nm-vpn-service-plugin.c @@ -10,6 +10,7 @@ #include #include +#include #include "libnm-glib-aux/nm-secret-utils.h" #include "libnm-glib-aux/nm-dbus-aux.h" @@ -728,6 +729,54 @@ nm_vpn_service_plugin_secrets_required(NMVpnServicePlugin *plugin, /*****************************************************************************/ +typedef struct { + char *buf; + gsize n_buf; + int fd; + bool eof : 1; + char buf_full[1024]; +} ReadFdBuf; + +static inline gboolean +_read_fd_buf_c(ReadFdBuf *read_buf, char *ch) +{ + gssize n_read; + + if (read_buf->n_buf > 0) + goto out_data; + if (read_buf->eof) + return FALSE; + +again: + n_read = read(read_buf->fd, read_buf->buf_full, sizeof(read_buf->buf_full)); + if (n_read <= 0) { + if (n_read < 0 && errno == EAGAIN) { + struct pollfd pfd; + int r; + + memset(&pfd, 0, sizeof(pfd)); + pfd.fd = read_buf->fd; + pfd.events = POLLIN; + + r = poll(&pfd, 1, -1); + if (r > 0) + goto again; + /* error or timeout. Fall through and set EOF. */ + } + read_buf->eof = TRUE; + return FALSE; + } + + read_buf->buf = read_buf->buf_full; + read_buf->n_buf = n_read; + +out_data: + read_buf->n_buf--; + *ch = read_buf->buf[0]; + read_buf->buf++; + return TRUE; +} + #define DATA_KEY_TAG "DATA_KEY=" #define DATA_VAL_TAG "DATA_VAL=" #define SECRET_KEY_TAG "SECRET_KEY=" @@ -758,9 +807,8 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable nm_auto_free_gstring GString *key = NULL; nm_auto_free_gstring GString *val = NULL; nm_auto_free_gstring GString *line = NULL; - char c; - - GString *str = NULL; + GString * str = NULL; + ReadFdBuf read_buf; if (out_data) g_return_val_if_fail(*out_data == NULL, FALSE); @@ -771,22 +819,32 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable secrets = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, (GDestroyNotify) nm_free_secret); + read_buf.buf = NULL; + read_buf.n_buf = 0; + read_buf.fd = fd; + read_buf.eof = FALSE; + line = g_string_new(NULL); /* Read stdin for data and secret items until we get a DONE */ while (1) { - ssize_t nr; + gboolean eof; + char c = '\0'; - nr = read(fd, &c, 1); - if (nr < 0) { - if (errno == EAGAIN) { - g_usleep(100); - continue; - } - break; + eof = !_read_fd_buf_c(&read_buf, &c); + + if (!eof && c == '\0') { + /* On the first '\0' char, we also assume the data is finished. Abort. */ + read_buf.eof = TRUE; + eof = TRUE; } - if (nr > 0 && c != '\n') { + + if (!eof && c != '\n') { g_string_append_c(line, c); + if (line->len > 512 * 1024) { + /* we are about to read a huge line. That is not right, abort. */ + break; + } continue; } @@ -794,7 +852,10 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable /* continuation */ g_string_append_c(str, '\n'); g_string_append(str, line->str + 1); - } else if (key && val) { + goto next; + } + + if (key && val) { /* done a line */ g_return_val_if_fail(hash, FALSE); g_hash_table_insert(hash, g_string_free(key, FALSE), g_string_free(val, FALSE)); @@ -804,48 +865,35 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable success = TRUE; /* Got at least one value */ } - if (strcmp(line->str, "DONE") == 0) { + if (nm_streq(line->str, "DONE")) { /* finish marker */ break; - } else if (strncmp(line->str, DATA_KEY_TAG, strlen(DATA_KEY_TAG)) == 0) { - if (key != NULL) { - g_warning("a value expected"); - g_string_free(key, TRUE); - } + } else if (NM_STR_HAS_PREFIX(line->str, DATA_KEY_TAG)) { + nm_clear_g_string(&key); key = g_string_new(line->str + strlen(DATA_KEY_TAG)); str = key; hash = data; - } else if (strncmp(line->str, DATA_VAL_TAG, strlen(DATA_VAL_TAG)) == 0) { - if (val != NULL) - g_string_free(val, TRUE); - if (val || !key || hash != data) { - g_warning("%s not preceded by %s", DATA_VAL_TAG, DATA_KEY_TAG); + } else if (NM_STR_HAS_PREFIX(line->str, DATA_VAL_TAG)) { + if (val || !key || hash != data) break; - } val = g_string_new(line->str + strlen(DATA_VAL_TAG)); str = val; - } else if (strncmp(line->str, SECRET_KEY_TAG, strlen(SECRET_KEY_TAG)) == 0) { - if (key != NULL) { - g_warning("a value expected"); - g_string_free(key, TRUE); - } + } else if (NM_STR_HAS_PREFIX(line->str, SECRET_KEY_TAG)) { + nm_clear_g_string(&key); key = g_string_new(line->str + strlen(SECRET_KEY_TAG)); str = key; hash = secrets; - } else if (strncmp(line->str, SECRET_VAL_TAG, strlen(SECRET_VAL_TAG)) == 0) { - if (val != NULL) - g_string_free(val, TRUE); - if (val || !key || hash != secrets) { - g_warning("%s not preceded by %s", SECRET_VAL_TAG, SECRET_KEY_TAG); + } else if (NM_STR_HAS_PREFIX(line->str, SECRET_VAL_TAG)) { + if (val || !key || hash != secrets) break; - } val = g_string_new(line->str + strlen(SECRET_VAL_TAG)); str = val; } +next: g_string_truncate(line, 0); - if (nr == 0) + if (eof) break; } diff --git a/src/libnm-client-impl/tests/test-libnm.c b/src/libnm-client-impl/tests/test-libnm.c index 003b030432..6cb8076e38 100644 --- a/src/libnm-client-impl/tests/test-libnm.c +++ b/src/libnm-client-impl/tests/test-libnm.c @@ -2462,8 +2462,7 @@ test_nm_vpn_service_plugin_read_vpn_details(void) "DONE\n" "", READ_VPN_DETAIL_DATA({"some-key", "string\ncontinued after a line break"}, ), - READ_VPN_DETAIL_DATA({"key names\ncan have\ncontinuations too", "value"}, ), - NMTST_EXPECT_LIBNM_WARNING("DATA_VAL= not preceded by DATA_KEY=")); + READ_VPN_DETAIL_DATA({"key names\ncan have\ncontinuations too", "value"}, ), ); _do_read_vpn_details( "" @@ -2478,43 +2477,77 @@ test_nm_vpn_service_plugin_read_vpn_details(void) READ_VPN_DETAIL_DATA({"some-key", "string\ncontinued after a line break"}, ), READ_VPN_DETAIL_DATA({"key names\ncan have\ncontinuations too", "value"}, ), ); - _do_read_vpn_details( - "" - "DATA_KEY=some-key\n" - "DATA_VAL=string\n" - "\n" - "DATA_KEY=some\n" - "=key-2\n" - "DATA_VAL=val2\n" - "\n" - "DATA_KEY=key3\0" - "=key-2\n" - "DATA_VAL=val3\n" - "\n" - "SECRET_KEY=some-secret\n" - "SECRET_VAL=val3\n" - "\n" - "SECRET_KEY=\n" - "SECRET_VAL=val3\n" - "\n" - "SECRET_KEY=keyx\n" - "SECRET_VAL=\n" - "\n" - "SECRET_KEY=ke\xc0yx\n" - "SECRET_VAL=inval\n" - "\n" - "SECRET_KEY=key-inval\n" - "SECRET_VAL=in\xc1val\n" - "\n" - "DONE\n" - "\n" - "", - READ_VPN_DETAIL_DATA({"some\nkey-2", "val2"}, {"some-key", "string"}, {"key3", "val3"}, ), - READ_VPN_DETAIL_DATA({"some-secret", "val3"}, - {"", "val3"}, - {"keyx", ""}, - {"ke\xc0yx", "inval"}, - {"key-inval", "in\xc1val"}, ), ); + _do_read_vpn_details("" + "DATA_KEY=some-key\n" + "DATA_VAL=string\n" + "\n" + "DATA_KEY=some\n" + "=key-2\n" + "DATA_VAL=val2\n" + "\n" + "DATA_KEY=key3\n" + "=key-2\n" + "DATA_VAL=val3\n" + "\n" + "SECRET_KEY=some-secret\n" + "SECRET_VAL=val3\n" + "\n" + "SECRET_KEY=\n" + "SECRET_VAL=val3\n" + "\n" + "SECRET_KEY=keyx\n" + "SECRET_VAL=\n" + "\n" + "SECRET_KEY=ke\xc0yx\n" + "SECRET_VAL=inval\n" + "\n" + "SECRET_KEY=key-inval\n" + "SECRET_VAL=in\xc1val\n" + "\n" + "DONE\n" + "\n" + "", + READ_VPN_DETAIL_DATA({"some\nkey-2", "val2"}, + {"some-key", "string"}, + {"key3\nkey-2", "val3"}, ), + READ_VPN_DETAIL_DATA({"some-secret", "val3"}, + {"", "val3"}, + {"keyx", ""}, + {"ke\xc0yx", "inval"}, + {"key-inval", "in\xc1val"}, ), ); + + _do_read_vpn_details("" + "DATA_KEY=some-key\n" + "DATA_VAL=string\n" + "\n" + "DATA_KEY=some\n" + "=key-2\n" + "DATA_VAL=val2\n" + "\n" + "DATA_KEY=key3\0" + "=key-2\n" + "DATA_VAL=val3\n" + "\n" + "SECRET_KEY=some-secret\n" + "SECRET_VAL=val3\n" + "\n" + "SECRET_KEY=\n" + "SECRET_VAL=val3\n" + "\n" + "SECRET_KEY=keyx\n" + "SECRET_VAL=\n" + "\n" + "SECRET_KEY=ke\xc0yx\n" + "SECRET_VAL=inval\n" + "\n" + "SECRET_KEY=key-inval\n" + "SECRET_VAL=in\xc1val\n" + "\n" + "DONE\n" + "\n" + "", + READ_VPN_DETAIL_DATA({"some\nkey-2", "val2"}, {"some-key", "string"}, ), + READ_VPN_DETAIL_DATA(), ); } /*****************************************************************************/ diff --git a/src/libnm-core-impl/tests/test-general.c b/src/libnm-core-impl/tests/test-general.c index 67b6ab482f..c911f0ec2c 100644 --- a/src/libnm-core-impl/tests/test-general.c +++ b/src/libnm-core-impl/tests/test-general.c @@ -1262,7 +1262,7 @@ test_nm_utils_escaped_tokens(void) num_options); g_print(">>> combined = \"%s\"\n", combined->str); g_print(">>> %c parsed[%5d].key = \"%s\"\n", - nm_streq(key, expected_key) ? ' ' : 'X', + nm_streq0(key, expected_key) ? ' ' : 'X', i_option, key); g_print(">>> %c parsed[%5d].val = %s%s%s\n", @@ -9102,6 +9102,7 @@ _do_test_utils_str_utf8safe_unescape(const char *str, const char *expected, gsiz /* there are no embedded NULs. Check that nm_utils_str_utf8safe_unescape() yields the same result. */ s = nm_utils_str_utf8safe_unescape(str, NM_UTILS_STR_UTF8_SAFE_FLAG_NONE, &str_free_1); g_assert_cmpstr(s, ==, expected); + g_assert(str); if (strchr(str, '\\')) { g_assert(str_free_1 != str); g_assert(s == str_free_1); diff --git a/src/libnm-glib-aux/nm-glib.h b/src/libnm-glib-aux/nm-glib.h index 44a7d19f82..66e5c16e14 100644 --- a/src/libnm-glib-aux/nm-glib.h +++ b/src/libnm-glib-aux/nm-glib.h @@ -693,4 +693,18 @@ _nm_g_cancellable_reset(GCancellable *cancellable); /*****************************************************************************/ +/* Coverity gets confused by g_strstrip(g_strdup(foo)). Reimplement the macro + * in a way that hopefully works better to avoid the false positive. */ +#undef g_strstrip +#define g_strstrip(str) \ + ({ \ + char *const _str = (str); \ + \ + g_strchug(_str); \ + g_strchomp(_str); \ + _str; \ + }) + +/*****************************************************************************/ + #endif /* __NM_GLIB_H__ */ diff --git a/src/libnm-glib-aux/nm-macros-internal.h b/src/libnm-glib-aux/nm-macros-internal.h index edaa6f3902..39197ecf3a 100644 --- a/src/libnm-glib-aux/nm-macros-internal.h +++ b/src/libnm-glib-aux/nm-macros-internal.h @@ -1136,6 +1136,18 @@ nm_clear_g_variant(GVariant **variant) return FALSE; } +static inline gboolean +nm_clear_g_string(GString **ptr) +{ + GString *s; + + if (ptr && (s = *ptr)) { + *ptr = NULL; + g_string_free(s, TRUE); + }; + return FALSE; +} + static inline gboolean nm_clear_g_cancellable(GCancellable **cancellable) { diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 26856cd1ee..2a7432751d 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -5555,7 +5555,7 @@ nm_str_buf_append_printf(NMStrBuf *strbuf, const char *format, ...) nm_assert(available < G_MAXULONG); va_start(args, format); - l = g_vsnprintf(strbuf->_priv_str ? &strbuf->_priv_str[strbuf->_priv_len] : NULL, + l = g_vsnprintf(strbuf->_priv_allocated > 0 ? &strbuf->_priv_str[strbuf->_priv_len] : NULL, available, format, args);