Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 704a5a0c authored by Lorenzo Colitti's avatar Lorenzo Colitti Committed by The Android Automerger
Browse files

Don't crash on (invalid) hardware address lengths > 127. 

These would cause us to crash with a NegativeArraySizeException
when trying to create the clientMac array. Instead, if the length
is > 16 (invalid, because the field is only 16 bytes long), fudge
it to 6 (Ethernet / wifi).  This is a bit less liberal than the
legacy client, which doesn't check the length at all.

Bug: 23725795
Change-Id: I83f47bfc400ffa8ce85dd9d1b8eb96be5afe51a5
parent e1d47a7b

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: