Merge "Don't crash on (invalid) hardware address lengths > 127." into mnc-dev (0a76afb9) · Commits · e / os / android_frameworks_base

services/net/java/android/net/dhcp/DhcpPacket.java

+13 −2

Original line number	Diff line number	Diff line
		@@ -55,6 +55,7 @@ abstract class DhcpPacket {
		public static final int MIN_PACKET_LENGTH_L3 = MIN_PACKET_LENGTH_BOOTP + 20 + 8;
		public static final int MIN_PACKET_LENGTH_L2 = MIN_PACKET_LENGTH_L3 + 14;

		public static final int HWADDR_LEN = 16;
		public static final int MAX_OPTION_LEN = 255;
		/**
		* IP layer definitions.
		@@ -399,7 +400,7 @@ abstract class DhcpPacket {
		buf.put(mRelayIp.getAddress());
		buf.put(mClientMac);
		buf.position(buf.position() +
		(16 - mClientMac.length) // pad addr to 16 bytes
		(HWADDR_LEN - mClientMac.length) // pad addr to 16 bytes
		+ 64 // empty server host name (64 bytes)
		+ 128); // empty boot file name (128 bytes)
		buf.putInt(0x63825363); // magic number
		@@ -786,7 +787,7 @@ abstract class DhcpPacket {

		byte type = packet.get();
		byte hwType = packet.get();
		byte addrLen = packet.get();
		int addrLen = packet.get() & 0xff;
		byte hops = packet.get();
		transactionId = packet.getInt();
		secs = packet.getShort();
		@@ -807,6 +808,16 @@ abstract class DhcpPacket {
		return null;
		}

		// Some DHCP servers have been known to announce invalid client hardware address values such
		// as 0xff. The legacy DHCP client accepted these becuause it does not check the length at
		// all but only checks that the interface MAC address matches the first bytes of the address
		// in the packets. We're a bit stricter: if the length is obviously invalid (i.e., bigger
		// than the size of the field), we fudge it to 6 (Ethernet). http://b/23725795
		// TODO: evaluate whether to make this test more liberal.
		if (addrLen > HWADDR_LEN) {
		addrLen = ETHER_BROADCAST.length;
		}

		clientMac = new byte[addrLen];
		packet.get(clientMac);

services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java

+70 −0

Original line number	Diff line number	Diff line
		@@ -332,6 +332,76 @@ public class DhcpPacketTest extends TestCase {
		assertTrue(dhcpResults.hasMeteredHint());
		}

		@SmallTest
		public void testBadHwaddrLength() throws Exception {
		final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
		// IP header.
		"450001518d0600004011144dc0a82b01c0a82bf7" +
		// UDP header.
		"00430044013d9ac7" +
		// BOOTP header.
		"02010600dfc23d1f0002000000000000c0a82bf7c0a82b0100000000" +
		// MAC address.
		"30766ff2a90c00000000000000000000" +
		// Server name.
		"0000000000000000000000000000000000000000000000000000000000000000" +
		"0000000000000000000000000000000000000000000000000000000000000000" +
		// File.
		"0000000000000000000000000000000000000000000000000000000000000000" +
		"0000000000000000000000000000000000000000000000000000000000000000" +
		"0000000000000000000000000000000000000000000000000000000000000000" +
		"0000000000000000000000000000000000000000000000000000000000000000" +
		// Options
		"638253633501023604c0a82b01330400000e103a04000007083b0400000c4e0104ffffff00" +
		"1c04c0a82bff0304c0a82b010604c0a82b012b0f414e44524f49445f4d455445524544ff"
		).toCharArray(), false));
		String expectedClientMac = "30766FF2A90C";

		final int hwAddrLenOffset = 20 + 8 + 2;
		assertEquals(6, packet.get(hwAddrLenOffset));

		// Expect the expected.
		DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
		assertNotNull(offerPacket);
		assertEquals(6, offerPacket.getClientMac().length);
		assertEquals(expectedClientMac, HexDump.toHexString(offerPacket.getClientMac()));

		// Reduce the hardware address length and verify that it shortens the client MAC.
		packet.flip();
		packet.put(hwAddrLenOffset, (byte) 5);
		offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
		assertNotNull(offerPacket);
		assertEquals(5, offerPacket.getClientMac().length);
		assertEquals(expectedClientMac.substring(0, 10),
		HexDump.toHexString(offerPacket.getClientMac()));

		packet.flip();
		packet.put(hwAddrLenOffset, (byte) 3);
		offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
		assertNotNull(offerPacket);
		assertEquals(3, offerPacket.getClientMac().length);
		assertEquals(expectedClientMac.substring(0, 6),
		HexDump.toHexString(offerPacket.getClientMac()));

		// Set the the hardware address length to 0xff and verify that we a) don't treat it as -1
		// and crash, and b) hardcode it to 6.
		packet.flip();
		packet.put(hwAddrLenOffset, (byte) -1);
		offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
		assertNotNull(offerPacket);
		assertEquals(6, offerPacket.getClientMac().length);
		assertEquals(expectedClientMac, HexDump.toHexString(offerPacket.getClientMac()));

		// Set the the hardware address length to a positive invalid value (> 16) and verify that we
		// hardcode it to 6.
		packet.flip();
		packet.put(hwAddrLenOffset, (byte) 17);
		offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
		assertNotNull(offerPacket);
		assertEquals(6, offerPacket.getClientMac().length);
		assertEquals(expectedClientMac, HexDump.toHexString(offerPacket.getClientMac()));
		}

		@SmallTest
		public void testPadAndOverloadedOptionsOffer() throws Exception {
		// A packet observed in the real world that is interesting for two reasons: