Merge "Don't crash on (invalid) hardware address lengths > 127." into mnc-dev

This commit is contained in:
Ian Pedowitz
2015-09-03 17:22:21 +00:00
committed by Android (Google) Code Review
2 changed files with 83 additions and 2 deletions

View File

@@ -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);

View File

@@ -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: