Bug#774898: fails to detect silent driver failure to change MAC

[Joey Hess]

Package: macchanger
Version: 1.7.0-5
Severity: normal
Tags: security

(Note that I have obscured my actal MAC with XX in this bug report.)

root@darkstar:~>macchanger wlan0 -m aa:bb:cc:dd:ee:ff
Current MAC:   40:f0:2f:XX:XX:XX (Liteon Technology Corporation)
Permanent MAC: 40:f0:2f:XX:XX:XX (Liteon Technology Corporation)
New MAC:       aa:bb:cc:dd:ee:ff (unknown)
root@darkstar:~>ifconfig wlan0
wlan0     Link encap:Ethernet  HWaddr 40:f0:2f:XX:XX:XX

It seems that the r8723au kernel driver for my wifi device silently
ignores attempts to change the MAC.

macchanger tries to detect if the new MAC is the same as the old.
However, it does this by calling mc_net_info_get_mac, on the same
net_info structure that it's called mc_net_info_set_mac on.

So, mc_net_info_set_mac sets the mac *in the data structure*, then calls
SIOCSIFHWADDR, which returns success without doing anything because the
kernel driver is buggy. And then mc_net_info_get_mac examines the data
structure, without calling SIOCGIFHWADDR to check if the MAC was
actually changed.

I suggest the attached patch to actively verify if the MAC was changed.

I'm tagging this security, because silently failing to change the MAC,
while claiming it was changed, may lead the user into trouble, if they
were changing their MAC for security reasons.

-- System Information:
Debian Release: 8.0
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.16.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.utf8, LC_CTYPE=en_US.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages macchanger depends on:
ii  debconf [debconf-2.0]  1.5.55
ii  dpkg                   1.17.23
ii  install-info           5.2.0.dfsg.1-6
ii  libc6                  2.19-13

macchanger recommends no packages.

macchanger suggests no packages.

-- debconf information excluded

-- 
see shy jo

diff --git a/src/main.c b/src/main.c
index 182082e..d57ce4d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -264,10 +264,6 @@ main (int argc, char *argv[])
        /* Set the new MAC */
        ret = mc_net_info_set_mac (net, mac_faked);
        if (ret == 0) {
-               /* Re-read the MAC */
-               mc_mac_free (mac_faked);
-               mac_faked = mc_net_info_get_mac(net);
-
                /* Print it */
                print_mac ("New MAC:       ", mac_faked);
 
@@ -276,6 +272,18 @@ main (int argc, char *argv[])
                        printf ("It's the same MAC!!\n");
                        exit (EXIT_ERROR);
                }
+               
+               /* Re-read the MAC in case a driver silently ignores
+                * attempt to set it */
+               mc_net_info_free (net);
+               if ((net = mc_net_info_new(device_name)) == NULL) {
+                       exit (EXIT_ERROR);
+               }
+               mac = mc_net_info_get_mac(net);
+               if (! mc_mac_equal (mac, mac_faked)) {
+                       printf ("Network driver didn't actually change to the 
new MAC!!\n");
+                       exit (EXIT_ERROR);
+               }
        }
 
        /* Memory free */

signature.asc
Description: Digital signature