core: refactor nm_utils_complete_generic() not to use a dynamic format string

For NMDeviceWifi and NMDeviceWimax, the printf format string for
nm_utils_complete_generic() was created based on ssid/nsp. Since
these input strings are untrusted, this is a serious bug.

Signed-off-by: Thomas Haller <thaller@redhat.com>
This commit is contained in:
Thomas Haller
2014-08-25 16:21:59 +02:00
parent 6de4a548df
commit ed20177d27
16 changed files with 39 additions and 31 deletions

View File

@@ -102,6 +102,7 @@ libnm/nm-object.c
libnm/nm-remote-connection.c libnm/nm-remote-connection.c
libnm/nm-vpn-plugin.c libnm/nm-vpn-plugin.c
policy/org.freedesktop.NetworkManager.policy.in.in policy/org.freedesktop.NetworkManager.policy.in.in
src/NetworkManagerUtils.c
src/main.c src/main.c
src/dhcp-manager/nm-dhcp-dhclient.c src/dhcp-manager/nm-dhcp-dhclient.c
src/dhcp-manager/nm-dhcp-dhclient-utils.c src/dhcp-manager/nm-dhcp-dhclient-utils.c

View File

@@ -19,7 +19,10 @@
* Copyright (C) 2005 - 2008 Novell, Inc. * Copyright (C) 2005 - 2008 Novell, Inc.
*/ */
#include "config.h"
#include <glib.h> #include <glib.h>
#include <glib/gi18n.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <string.h> #include <string.h>
@@ -846,8 +849,8 @@ value_hash_add_object_property (GHashTable *hash,
static char * static char *
get_new_connection_name (const GSList *existing, get_new_connection_name (const GSList *existing,
const char *format, const char *preferred,
const char *preferred) const char *fallback_prefix)
{ {
GSList *names = NULL; GSList *names = NULL;
const GSList *iter; const GSList *iter;
@@ -855,6 +858,8 @@ get_new_connection_name (const GSList *existing,
int i = 0; int i = 0;
gboolean preferred_found = FALSE; gboolean preferred_found = FALSE;
g_assert (fallback_prefix);
for (iter = existing; iter; iter = g_slist_next (iter)) { for (iter = existing; iter; iter = g_slist_next (iter)) {
NMConnection *candidate = NM_CONNECTION (iter->data); NMConnection *candidate = NM_CONNECTION (iter->data);
const char *id; const char *id;
@@ -880,7 +885,12 @@ get_new_connection_name (const GSList *existing,
char *temp; char *temp;
gboolean found = FALSE; gboolean found = FALSE;
temp = g_strdup_printf (format, i); /* Translators: the first %s is a prefix for the connection id, such
* as "Wired Connection" or "VPN Connection". The %d is a number
* that is combined with the first argument to create a unique
* connection id. */
temp = g_strdup_printf (C_("connection id fallback", "%s %d"),
fallback_prefix, i);
for (iter = names; iter; iter = g_slist_next (iter)) { for (iter = names; iter; iter = g_slist_next (iter)) {
if (!strcmp (iter->data, temp)) { if (!strcmp (iter->data, temp)) {
found = TRUE; found = TRUE;
@@ -944,14 +954,16 @@ void
nm_utils_complete_generic (NMConnection *connection, nm_utils_complete_generic (NMConnection *connection,
const char *ctype, const char *ctype,
const GSList *existing, const GSList *existing,
const char *format,
const char *preferred, const char *preferred,
const char *fallback_prefix,
gboolean default_enable_ipv6) gboolean default_enable_ipv6)
{ {
NMSettingConnection *s_con; NMSettingConnection *s_con;
char *id, *uuid; char *id, *uuid;
GHashTable *parameters = g_hash_table_new (g_str_hash, g_str_equal); GHashTable *parameters = g_hash_table_new (g_str_hash, g_str_equal);
g_assert (fallback_prefix);
g_hash_table_insert (parameters, NM_CONNECTION_NORMALIZE_PARAM_IP6_CONFIG_METHOD, g_hash_table_insert (parameters, NM_CONNECTION_NORMALIZE_PARAM_IP6_CONFIG_METHOD,
default_enable_ipv6 ? NM_SETTING_IP6_CONFIG_METHOD_AUTO : NM_SETTING_IP6_CONFIG_METHOD_IGNORE); default_enable_ipv6 ? NM_SETTING_IP6_CONFIG_METHOD_AUTO : NM_SETTING_IP6_CONFIG_METHOD_IGNORE);
@@ -970,7 +982,7 @@ nm_utils_complete_generic (NMConnection *connection,
/* Add a connection ID if absent */ /* Add a connection ID if absent */
if (!nm_setting_connection_get_id (s_con)) { if (!nm_setting_connection_get_id (s_con)) {
id = get_new_connection_name (existing, format, preferred); id = get_new_connection_name (existing, preferred, fallback_prefix);
g_object_set (G_OBJECT (s_con), NM_SETTING_CONNECTION_ID, id, NULL); g_object_set (G_OBJECT (s_con), NM_SETTING_CONNECTION_ID, id, NULL);
g_free (id); g_free (id);
} }

View File

@@ -102,8 +102,8 @@ const char *nm_utils_get_ip_config_method (NMConnection *connection,
void nm_utils_complete_generic (NMConnection *connection, void nm_utils_complete_generic (NMConnection *connection,
const char *ctype, const char *ctype,
const GSList *existing, const GSList *existing,
const char *format,
const char *preferred, const char *preferred,
const char *fallback_prefix,
gboolean default_enable_ipv6); gboolean default_enable_ipv6);
char *nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id); char *nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id);

View File

@@ -123,8 +123,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_ADSL_SETTING_NAME, NM_SETTING_ADSL_SETTING_NAME,
existing_connections, existing_connections,
_("ADSL connection %d"),
NULL, NULL,
_("ADSL connection"),
FALSE); /* No IPv6 yet by default */ FALSE); /* No IPv6 yet by default */

View File

@@ -222,7 +222,7 @@ complete_connection (NMDevice *device,
NMSettingCdma *s_cdma; NMSettingCdma *s_cdma;
NMSettingSerial *s_serial; NMSettingSerial *s_serial;
NMSettingPpp *s_ppp; NMSettingPpp *s_ppp;
const char *format = NULL, *preferred = NULL; const char *fallback_prefix = NULL, *preferred = NULL;
s_gsm = nm_connection_get_setting_gsm (connection); s_gsm = nm_connection_get_setting_gsm (connection);
s_cdma = nm_connection_get_setting_cdma (connection); s_cdma = nm_connection_get_setting_cdma (connection);
@@ -271,7 +271,7 @@ complete_connection (NMDevice *device,
NM_SETTING_BLUETOOTH_TYPE, NM_SETTING_BLUETOOTH_TYPE_PANU, NM_SETTING_BLUETOOTH_TYPE, NM_SETTING_BLUETOOTH_TYPE_PANU,
NULL); NULL);
format = _("PAN connection %d"); fallback_prefix = _("PAN connection");
} else if (is_dun) { } else if (is_dun) {
/* Make sure the device supports PAN */ /* Make sure the device supports PAN */
if (!(priv->capabilities & NM_BT_CAPABILITY_DUN)) { if (!(priv->capabilities & NM_BT_CAPABILITY_DUN)) {
@@ -296,15 +296,15 @@ complete_connection (NMDevice *device,
NULL); NULL);
if (s_gsm) { if (s_gsm) {
format = _("GSM connection %d"); fallback_prefix = _("GSM connection");
if (!nm_setting_gsm_get_number (s_gsm)) if (!nm_setting_gsm_get_number (s_gsm))
g_object_set (G_OBJECT (s_gsm), NM_SETTING_GSM_NUMBER, "*99#", NULL); g_object_set (G_OBJECT (s_gsm), NM_SETTING_GSM_NUMBER, "*99#", NULL);
} else if (s_cdma) { } else if (s_cdma) {
format = _("CDMA connection %d"); fallback_prefix = _("CDMA connection");
if (!nm_setting_cdma_get_number (s_cdma)) if (!nm_setting_cdma_get_number (s_cdma))
g_object_set (G_OBJECT (s_cdma), NM_SETTING_GSM_NUMBER, "#777", NULL); g_object_set (G_OBJECT (s_cdma), NM_SETTING_GSM_NUMBER, "#777", NULL);
} else } else
format = _("DUN connection %d"); fallback_prefix = _("DUN connection");
} else { } else {
g_set_error_literal (error, g_set_error_literal (error,
NM_SETTING_BLUETOOTH_ERROR, NM_SETTING_BLUETOOTH_ERROR,
@@ -316,8 +316,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_BLUETOOTH_SETTING_NAME, NM_SETTING_BLUETOOTH_SETTING_NAME,
existing_connections, existing_connections,
format,
preferred, preferred,
fallback_prefix,
is_dun ? FALSE : TRUE); /* No IPv6 yet for DUN */ is_dun ? FALSE : TRUE); /* No IPv6 yet for DUN */
setting_bdaddr = nm_setting_bluetooth_get_bdaddr (s_bt); setting_bdaddr = nm_setting_bluetooth_get_bdaddr (s_bt);

View File

@@ -136,8 +136,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_SETTING_NAME,
existing_connections, existing_connections,
_("Bond connection %d"),
NULL, NULL,
_("Bond connection"),
TRUE); TRUE);
s_bond = nm_connection_get_setting_bond (connection); s_bond = nm_connection_get_setting_bond (connection);

View File

@@ -144,8 +144,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_BRIDGE_SETTING_NAME, NM_SETTING_BRIDGE_SETTING_NAME,
existing_connections, existing_connections,
_("Bridge connection %d"),
NULL, NULL,
_("Bridge connection"),
TRUE); TRUE);
s_bridge = nm_connection_get_setting_bridge (connection); s_bridge = nm_connection_get_setting_bridge (connection);

View File

@@ -1446,8 +1446,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
s_pppoe ? NM_SETTING_PPPOE_SETTING_NAME : NM_SETTING_WIRED_SETTING_NAME, s_pppoe ? NM_SETTING_PPPOE_SETTING_NAME : NM_SETTING_WIRED_SETTING_NAME,
existing_connections, existing_connections,
s_pppoe ? _("PPPoE connection %d") : _("Wired connection %d"),
NULL, NULL,
s_pppoe ? _("PPPoE connection") : _("Wired connection"),
s_pppoe ? FALSE : TRUE); /* No IPv6 by default yet for PPPoE */ s_pppoe ? FALSE : TRUE); /* No IPv6 by default yet for PPPoE */
s_wired = nm_connection_get_setting_wired (connection); s_wired = nm_connection_get_setting_wired (connection);

View File

@@ -230,8 +230,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_INFINIBAND_SETTING_NAME, NM_SETTING_INFINIBAND_SETTING_NAME,
existing_connections, existing_connections,
_("InfiniBand connection %d"),
NULL, NULL,
_("InfiniBand connection"),
TRUE); TRUE);
s_infiniband = nm_connection_get_setting_infiniband (connection); s_infiniband = nm_connection_get_setting_infiniband (connection);

View File

@@ -226,8 +226,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_SETTING_NAME,
existing_connections, existing_connections,
_("VLAN connection %d"),
NULL, NULL,
_("VLAN connection"),
TRUE); TRUE);
s_vlan = nm_connection_get_setting_vlan (connection); s_vlan = nm_connection_get_setting_vlan (connection);

View File

@@ -147,8 +147,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_TEAM_SETTING_NAME, NM_SETTING_TEAM_SETTING_NAME,
existing_connections, existing_connections,
_("Team connection %d"),
NULL, NULL,
_("Team connection"),
TRUE); TRUE);
s_team = nm_connection_get_setting_team (connection); s_team = nm_connection_get_setting_team (connection);

View File

@@ -162,8 +162,8 @@ complete_connection (NMDevice *device,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_OLPC_MESH_SETTING_NAME, NM_SETTING_OLPC_MESH_SETTING_NAME,
existing_connections, existing_connections,
_("Mesh %d"),
NULL, NULL,
_("Mesh"),
FALSE); /* No IPv6 by default */ FALSE); /* No IPv6 by default */
return TRUE; return TRUE;

View File

@@ -981,7 +981,7 @@ complete_connection (NMDevice *device,
NMSettingWirelessSecurity *s_wsec; NMSettingWirelessSecurity *s_wsec;
NMSetting8021x *s_8021x; NMSetting8021x *s_8021x;
const GByteArray *setting_mac; const GByteArray *setting_mac;
char *format, *str_ssid = NULL; char *str_ssid = NULL;
NMAccessPoint *ap = NULL; NMAccessPoint *ap = NULL;
const GByteArray *ssid = NULL; const GByteArray *ssid = NULL;
GSList *iter; GSList *iter;
@@ -1103,16 +1103,14 @@ complete_connection (NMDevice *device,
g_assert (ssid); g_assert (ssid);
str_ssid = nm_utils_ssid_to_utf8 (ssid); str_ssid = nm_utils_ssid_to_utf8 (ssid);
format = g_strdup_printf ("%s %%d", str_ssid);
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_WIRELESS_SETTING_NAME, NM_SETTING_WIRELESS_SETTING_NAME,
existing_connections, existing_connections,
format, str_ssid,
str_ssid, str_ssid,
TRUE); TRUE);
g_free (str_ssid); g_free (str_ssid);
g_free (format);
if (hidden) if (hidden)
g_object_set (s_wifi, NM_SETTING_WIRELESS_HIDDEN, TRUE, NULL); g_object_set (s_wifi, NM_SETTING_WIRELESS_HIDDEN, TRUE, NULL);

View File

@@ -373,7 +373,6 @@ complete_connection (NMDevice *device,
NMSettingWimax *s_wimax; NMSettingWimax *s_wimax;
const GByteArray *setting_mac; const GByteArray *setting_mac;
const char *hw_address; const char *hw_address;
char *format;
const char *nsp_name = NULL; const char *nsp_name = NULL;
NMWimaxNsp *nsp = NULL; NMWimaxNsp *nsp = NULL;
GSList *iter; GSList *iter;
@@ -438,14 +437,12 @@ complete_connection (NMDevice *device,
} }
g_assert (nsp_name); g_assert (nsp_name);
format = g_strdup_printf ("%s %%d", nsp_name);
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_WIMAX_SETTING_NAME, NM_SETTING_WIMAX_SETTING_NAME,
existing_connections, existing_connections,
format, nsp_name,
nsp_name, nsp_name,
TRUE); TRUE);
g_free (format);
g_object_set (G_OBJECT (s_wimax), NM_SETTING_WIMAX_NETWORK_NAME, nsp_name, NULL); g_object_set (G_OBJECT (s_wimax), NM_SETTING_WIMAX_NETWORK_NAME, nsp_name, NULL);
setting_mac = nm_setting_wimax_get_mac_address (s_wimax); setting_mac = nm_setting_wimax_get_mac_address (s_wimax);

View File

@@ -479,8 +479,8 @@ complete_connection (NMModem *_self,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_GSM_SETTING_NAME, NM_SETTING_GSM_SETTING_NAME,
existing_connections, existing_connections,
_("GSM connection %d"),
NULL, NULL,
_("GSM connection"),
FALSE); /* No IPv6 yet by default */ FALSE); /* No IPv6 yet by default */
return TRUE; return TRUE;
@@ -501,8 +501,8 @@ complete_connection (NMModem *_self,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_CDMA_SETTING_NAME, NM_SETTING_CDMA_SETTING_NAME,
existing_connections, existing_connections,
_("CDMA connection %d"),
NULL, NULL,
_("CDMA connection"),
FALSE); /* No IPv6 yet by default */ FALSE); /* No IPv6 yet by default */
return TRUE; return TRUE;

View File

@@ -3435,8 +3435,8 @@ impl_manager_add_and_activate_connection (NMManager *self,
nm_utils_complete_generic (connection, nm_utils_complete_generic (connection,
NM_SETTING_VPN_SETTING_NAME, NM_SETTING_VPN_SETTING_NAME,
all_connections, all_connections,
_("VPN connection %d"),
NULL, NULL,
_("VPN connection"),
FALSE); /* No IPv6 by default for now */ FALSE); /* No IPv6 by default for now */
} else { } else {
/* Let each device subclass complete the connection */ /* Let each device subclass complete the connection */