Commit abe84859 authored by Thomas Haller's avatar Thomas Haller

keyfile: ensure array lengths are always initialized

Several callers access the length output argument, expecting
it to be zero also on failure. That is a bug, because glib does
not guarantee that.

Fix that by making a stronger promise from our wrappers: the output
argument should also be set on failure.

Also ensure that calls to g_keyfile_get_groups() and
g_keyfile_get_keys() don't rely on the length output to be valid,
when the function call fails.

Actually, these issues were not severe because:

- `g_key_file_get_*_list()`'s implementation always sets the output length
  even on failure (undocumented).
- `g_key_file_get_groups()` cannot fail and always set the length.
- `g_key_file_get_keys()` is called under circumstances where it won't
  fail.

Still, be explicit about it.
parent cce3c0c6
......@@ -82,17 +82,22 @@ nm_keyfile_plugin_kf_get_##stype##_list (GKeyFile *kf, \
get_ctype list; \
const char *alias; \
GError *local = NULL; \
gsize l; \
\
list = g_key_file_get_##stype##_list (kf, group, key, out_length, &local); \
list = g_key_file_get_##stype##_list (kf, group, key, &l, &local); \
if (g_error_matches (local, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND)) { \
alias = nm_keyfile_plugin_get_alias_for_setting_name (group); \
if (alias) { \
g_clear_error (&local); \
list = g_key_file_get_##stype##_list (kf, alias, key, out_length, &local); \
list = g_key_file_get_##stype##_list (kf, alias, key, &l, &local); \
} \
} \
nm_assert ((!local) != (!list)); \
if (local) \
g_propagate_error (error, local); \
if (!list) \
l = 0; \
NM_SET_OUT (out_length, l); \
return list; \
} \
\
......@@ -188,15 +193,21 @@ nm_keyfile_plugin_kf_get_keys (GKeyFile *kf,
char **keys;
const char *alias;
GError *local = NULL;
gsize l;
keys = g_key_file_get_keys (kf, group, out_length, &local);
keys = g_key_file_get_keys (kf, group, &l, &local);
if (g_error_matches (local, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND)) {
alias = nm_keyfile_plugin_get_alias_for_setting_name (group);
if (alias) {
g_clear_error (&local);
keys = g_key_file_get_keys (kf, alias, out_length, error ? &local : NULL);
keys = g_key_file_get_keys (kf, alias, &l, error ? &local : NULL);
}
}
nm_assert ((!local) != (!keys));
if (!keys)
l = 0;
nm_assert (l == NM_PTRARRAY_LEN (keys));
NM_SET_OUT (out_length, l);
if (local)
g_propagate_error (error, local);
return keys;
......
......@@ -2723,6 +2723,8 @@ read_setting (KeyfileReaderInfo *info)
gsize i, n_keys;
keys = g_key_file_get_keys (info->keyfile, info->group, &n_keys, NULL);
if (!keys)
n_keys = 0;
if (n_keys > 0) {
GHashTable *h = _nm_setting_gendata_hash (setting, TRUE);
......@@ -2798,15 +2800,15 @@ static void
read_vpn_secrets (KeyfileReaderInfo *info, NMSettingVpn *s_vpn)
{
gs_strfreev char **keys = NULL;
char **iter;
gsize i, n_keys;
keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, NULL, NULL);
for (iter = keys; *iter; iter++) {
keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, &n_keys, NULL);
for (i = 0; i < n_keys; i++) {
char *secret;
secret = nm_keyfile_plugin_kf_get_string (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, *iter, NULL);
secret = nm_keyfile_plugin_kf_get_string (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, keys[i], NULL);
if (secret) {
nm_setting_vpn_add_secret (s_vpn, *iter, secret);
nm_setting_vpn_add_secret (s_vpn, keys[i], secret);
g_free (secret);
}
}
......@@ -2896,6 +2898,9 @@ nm_keyfile_read (GKeyFile *keyfile,
info.user_data = user_data;
groups = g_key_file_get_groups (keyfile, &length);
if (!groups)
length = 0;
for (i = 0; i < length; i++) {
/* Only read out secrets when needed */
if (!strcmp (groups[i], NM_KEYFILE_GROUP_VPN_SECRETS)) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment