I've merged bugs 740947 and 738460 because they are fundamentally the
same bug.  I suspect this was caused by a mistake while upgrading the
patch 08-fix_random_MAC_choice.patch when upgrading to the upstream
1.7.0 release.

Now instead of fixing the random MAC choice, it is completely breaking
it.  After creating a random MAC address, the patch is now causing the
mc_mac_random() function to overwrite the freshly created mac address
with the original mac address.  :-(

Since this fundamentally breaks the functionality of the macchanger
package, I've upgraded the severity of the bugs to "grave".  The
mac.c.patch contains the necessary fix to mac.c, and the
0001-Fix-random-mac-address-setting-which-was-completely-.patch
attachment contains a patch suitable for application via "git am" to
the git repository.

Do you have any objections if I upload this as a NMU?  Or would you
prefer to update the package?

                                                - Ted

--- src/mac.c.orig	2014-07-06 20:30:55.499840061 -0400
+++ src/mac.c	2014-07-06 20:31:25.319447245 -0400
@@ -75,8 +75,8 @@
 	 * x1:, x3:, x5:, x7:, x9:, xB:, xD: and xF:
 	 */
 
-	mac_t newmac;
-	mc_mac_copy(mac, &newmac);
+	mac_t origmac;
+	mc_mac_copy(mac, &origmac);
 
 	do {
 		switch (last_n_bytes) {
@@ -100,9 +100,7 @@
 		} else {
 			mac->byte[0] |= 2;
 		}
-	} while (mc_mac_equal (&newmac, mac));
-
-	mc_mac_copy(&newmac, mac);
+	} while (mc_mac_equal (&origmac, mac));
 }
 
 
>From e7c13f36b96d6e03e865308cc5690ca18fd9e290 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <ty...@mit.edu>
Date: Sun, 6 Jul 2014 20:37:37 -0400
Subject: [PATCH] Fix random mac address setting, which was completely broken

Addresses-Debian-Bug: #738460, #740947
Signed-off-by: Theodore Ts'o <ty...@mit.edu>
---
 debian/changelog                              | 10 ++++++
 debian/patches/08-fix_random_MAC_choice.patch | 49 ++++++++++++++-------------
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 074365d..27a49e5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+macchanger (1.7.0-1.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Fix a grave security bug -- the macchanger program is fundmantally
+    was not working correctly due to a bug in the debian local patch
+    08-fix_random_MAC_choice.patch.   In fact, it was **breaking** the
+    random MAC choice!?! (Closes: #738460, #740947)
+
+ -- Theodore Y. Ts'o <ty...@mit.edu>  Sun, 06 Jul 2014 20:32:38 -0400
+
 macchanger (1.7.0-1) unstable; urgency=medium
 
   * New upstream release (Closes: #718849)
diff --git a/debian/patches/08-fix_random_MAC_choice.patch b/debian/patches/08-fix_random_MAC_choice.patch
index d3ba14d..54ccfb1 100644
--- a/debian/patches/08-fix_random_MAC_choice.patch
+++ b/debian/patches/08-fix_random_MAC_choice.patch
@@ -12,6 +12,8 @@ Subject: ensure random new MAC is not same as old MAC
  src/main.c |    1 +
  2 files changed, 34 insertions(+), 19 deletions(-)
 
+Index: macchanger/src/mac.c
+===================================================================
 --- macchanger.orig/src/mac.c
 +++ macchanger/src/mac.c
 @@ -41,6 +41,13 @@ mc_mac_dup (const mac_t *mac)
@@ -28,7 +30,7 @@ Subject: ensure random new MAC is not same as old MAC
  
  void
  mc_mac_free (mac_t *mac)
-@@ -68,27 +75,34 @@ mc_mac_random (mac_t *mac, unsigned char
+@@ -68,27 +75,32 @@ mc_mac_random (mac_t *mac, unsigned char
  	 * x1:, x3:, x5:, x7:, x9:, xB:, xD: and xF:
  	 */
  
@@ -36,9 +38,25 @@ Subject: ensure random new MAC is not same as old MAC
 -	case 6:
 -		/* 8th bit: Unicast / Multicast address
 -		 * 7th bit: BIA (burned-in-address) / locally-administered
-+	mac_t newmac;
-+	mc_mac_copy(mac, &newmac);
-+
+-		 */
+-		mac->byte[0] = (random()%255) & 0xFC;
+-		mac->byte[1] = random()%255;
+-		mac->byte[2] = random()%255;
+-	case 3:
+-		mac->byte[3] = random()%255;
+-		mac->byte[4] = random()%255;
+-		mac->byte[5] = random()%255;
+-	}
++	mac_t origmac;
++	mc_mac_copy(mac, &origmac);
+ 
+-	/* Handle the burned-in-address bit
+-	 */
+-	if (set_bia) {
+-		mac->byte[0] &= ~2;
+-	} else {
+-		mac->byte[0] |= 2;
+-	}
 +	do {
 +		switch (last_n_bytes) {
 +		case 6:
@@ -55,33 +73,18 @@ Subject: ensure random new MAC is not same as old MAC
 +		}
 +
 +		/* Handle the burned-in-address bit
- 		 */
--		mac->byte[0] = (random()%255) & 0xFC;
--		mac->byte[1] = random()%255;
--		mac->byte[2] = random()%255;
--	case 3:
--		mac->byte[3] = random()%255;
--		mac->byte[4] = random()%255;
--		mac->byte[5] = random()%255;
--	}
++		 */
 +		if (set_bia) {
 +			mac->byte[0] &= ~2;
 +		} else {
 +			mac->byte[0] |= 2;
 +		}
-+	} while (mc_mac_equal (&newmac, mac));
- 
--	/* Handle the burned-in-address bit
--	 */
--	if (set_bia) {
--		mac->byte[0] &= ~2;
--	} else {
--		mac->byte[0] |= 2;
--	}
-+	mc_mac_copy(&newmac, mac);
++	} while (mc_mac_equal (&origmac, mac));
  }
  
  
+Index: macchanger/src/main.c
+===================================================================
 --- macchanger.orig/src/main.c
 +++ macchanger/src/main.c
 @@ -274,6 +274,7 @@ main (int argc, char *argv[])
-- 
2.0.0

Reply via email to