Merge "Don't crash on (invalid) hardware address lengths > 127." into mnc-dev
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user