DHCP: parsing robustness fixes.

1. Check the length of the fixed-length portions of the packet.
2. Catch BufferUnderflowException while parsing options.

Change-Id: If907f49f02a04a4a3360f46a3192e94ab099af0e

1 files changed, 123 insertions, 105 deletions

diff --git a/services/net/java/android/net/dhcp/DhcpPacket.java b/services/net/java/android/net/dhcp/DhcpPacket.java
index fd18513..770f5c1 100644
--- a/services/net/java/android/net/dhcp/DhcpPacket.java
+++ b/services/net/java/android/net/dhcp/DhcpPacket.java
@@ -42,6 +42,13 @@ abstract class DhcpPacket {
     public static final int ENCAP_BOOTP = 2; // BOOTP contents only
 
     /**
+     * Minimum length of a DHCP packet, excluding options, in the above encapsulations.
+     */
+    public static final int MIN_PACKET_LENGTH_BOOTP = 236;  // See diagram in RFC 2131, section 2.
+    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;
+
+    /**
      * IP layer definitions.
      */
     private static final byte IP_TYPE_UDP = (byte) 0x11;
@@ -399,13 +406,8 @@ abstract class DhcpPacket {
      * Converts a signed short value to an unsigned int value.  Needed
      * because Java does not have unsigned types.
      */
-    private int intAbs(short v) {
-        if (v < 0) {
-            int r = v + 65536;
-            return r;
-        } else {
-            return(v);
-        }
+    private static int intAbs(short v) {
+        return v & 0xFFFF;
     }
 
     /**
@@ -655,6 +657,10 @@ abstract class DhcpPacket {
 
         // check to see if we need to parse L2, IP, and UDP encaps
         if (pktType == ENCAP_L2) {
+            if (packet.remaining() < MIN_PACKET_LENGTH_L2) {
+                return null;
+            }
+
             byte[] l2dst = new byte[6];
             byte[] l2src = new byte[6];
 
@@ -668,6 +674,10 @@ abstract class DhcpPacket {
         }
 
         if (pktType <= ENCAP_L3) {
+            if (packet.remaining() < MIN_PACKET_LENGTH_L3) {
+                return null;
+            }
+
             byte ipTypeAndLength = packet.get();
             int ipVersion = (ipTypeAndLength & 0xf0) >> 4;
             if (ipVersion != 4) {
@@ -690,7 +700,9 @@ abstract class DhcpPacket {
             if (ipProto != IP_TYPE_UDP) // UDP
                 return null;
 
-            // Skip options.
+            // Skip options. This cannot cause us to read beyond the end of the buffer because the
+            // IPv4 header cannot be more than (0x0f * 4) = 60 bytes long, and that is less than
+            // MIN_PACKET_LENGTH_L3.
             int optionWords = ((ipTypeAndLength & 0x0f) - 5);
             for (int i = 0; i < optionWords; i++) {
                 packet.getInt();
@@ -706,6 +718,11 @@ abstract class DhcpPacket {
                 return null;
         }
 
+        // We need to check the length even for ENCAP_L3 because the IPv4 header is variable-length.
+        if (pktType > ENCAP_BOOTP || packet.remaining() < MIN_PACKET_LENGTH_BOOTP) {
+            return null;
+        }
+
         byte type = packet.get();
         byte hwType = packet.get();
         byte addrLen = packet.get();
@@ -746,104 +763,105 @@ abstract class DhcpPacket {
         boolean notFinishedOptions = true;
 
         while ((packet.position() < packet.limit()) && notFinishedOptions) {
-            byte optionType = packet.get();
-
-            if (optionType == (byte) 0xFF) {
-                notFinishedOptions = false;
-            } else {
-                byte optionLen = packet.get();
-                int expectedLen = 0;
-
-                switch(optionType) {
-                    case DHCP_SUBNET_MASK:
-                        netMask = readIpAddress(packet);
-                        expectedLen = 4;
-                        break;
-                    case DHCP_ROUTER:
-                        gateway = readIpAddress(packet);
-                        expectedLen = 4;
-                        break;
-                    case DHCP_DNS_SERVER:
-                        expectedLen = 0;
-
-                        for (expectedLen = 0; expectedLen < optionLen;
-                             expectedLen += 4) {
-                            dnsServers.add(readIpAddress(packet));
-                        }
-                        break;
-                    case DHCP_HOST_NAME:
-                        expectedLen = optionLen;
-                        hostName = readAsciiString(packet, optionLen);
-                        break;
-                    case DHCP_MTU:
-                        expectedLen = 2;
-                        mtu = Short.valueOf(packet.getShort());
-                        break;
-                    case DHCP_DOMAIN_NAME:
-                        expectedLen = optionLen;
-                        domainName = readAsciiString(packet, optionLen);
-                        break;
-                    case DHCP_BROADCAST_ADDRESS:
-                        bcAddr = readIpAddress(packet);
-                        expectedLen = 4;
-                        break;
-                    case DHCP_REQUESTED_IP:
-                        requestedIp = readIpAddress(packet);
-                        expectedLen = 4;
-                        break;
-                    case DHCP_LEASE_TIME:
-                        leaseTime = Integer.valueOf(packet.getInt());
-                        expectedLen = 4;
-                        break;
-                    case DHCP_MESSAGE_TYPE:
-                        dhcpType = packet.get();
-                        expectedLen = 1;
-                        break;
-                    case DHCP_SERVER_IDENTIFIER:
-                        serverIdentifier = readIpAddress(packet);
-                        expectedLen = 4;
-                        break;
-                    case DHCP_PARAMETER_LIST:
-                        expectedParams = new byte[optionLen];
-                        packet.get(expectedParams);
-                        expectedLen = optionLen;
-                        break;
-                    case DHCP_MESSAGE:
-                        expectedLen = optionLen;
-                        message = readAsciiString(packet, optionLen);
-                        break;
-                    case DHCP_MAX_MESSAGE_SIZE:
-                        expectedLen = 2;
-                        maxMessageSize = Short.valueOf(packet.getShort());
-                        break;
-                    case DHCP_RENEWAL_TIME:
-                        expectedLen = 4;
-                        T1 = Integer.valueOf(packet.getInt());
-                        break;
-                    case DHCP_REBINDING_TIME:
-                        expectedLen = 4;
-                        T2 = Integer.valueOf(packet.getInt());
-                        break;
-                    case DHCP_VENDOR_CLASS_ID:
-                        expectedLen = optionLen;
-                        vendorId = readAsciiString(packet, optionLen);
-                        break;
-                    case DHCP_CLIENT_IDENTIFIER: { // Client identifier
-                        byte[] id = new byte[optionLen];
-                        packet.get(id);
-                        expectedLen = optionLen;
-                    } break;
-                    default:
-                        // ignore any other parameters
-                        for (int i = 0; i < optionLen; i++) {
-                            expectedLen++;
-                            byte throwaway = packet.get();
-                        }
-                }
-
-                if (expectedLen != optionLen) {
-                    return null;
+            try {
+                byte optionType = packet.get();
+
+                if (optionType == (byte) 0xFF) {
+                    notFinishedOptions = false;
+                } else {
+                    int optionLen = packet.get() & 0xFF;
+                    int expectedLen = 0;
+
+                    switch(optionType) {
+                        case DHCP_SUBNET_MASK:
+                            netMask = readIpAddress(packet);
+                            expectedLen = 4;
+                            break;
+                        case DHCP_ROUTER:
+                            gateway = readIpAddress(packet);
+                            expectedLen = 4;
+                            break;
+                        case DHCP_DNS_SERVER:
+                            for (expectedLen = 0; expectedLen < optionLen; expectedLen += 4) {
+                                dnsServers.add(readIpAddress(packet));
+                            }
+                            break;
+                        case DHCP_HOST_NAME:
+                            expectedLen = optionLen;
+                            hostName = readAsciiString(packet, optionLen);
+                            break;
+                        case DHCP_MTU:
+                            expectedLen = 2;
+                            mtu = Short.valueOf(packet.getShort());
+                            break;
+                        case DHCP_DOMAIN_NAME:
+                            expectedLen = optionLen;
+                            domainName = readAsciiString(packet, optionLen);
+                            break;
+                        case DHCP_BROADCAST_ADDRESS:
+                            bcAddr = readIpAddress(packet);
+                            expectedLen = 4;
+                            break;
+                        case DHCP_REQUESTED_IP:
+                            requestedIp = readIpAddress(packet);
+                            expectedLen = 4;
+                            break;
+                        case DHCP_LEASE_TIME:
+                            leaseTime = Integer.valueOf(packet.getInt());
+                            expectedLen = 4;
+                            break;
+                        case DHCP_MESSAGE_TYPE:
+                            dhcpType = packet.get();
+                            expectedLen = 1;
+                            break;
+                        case DHCP_SERVER_IDENTIFIER:
+                            serverIdentifier = readIpAddress(packet);
+                            expectedLen = 4;
+                            break;
+                        case DHCP_PARAMETER_LIST:
+                            expectedParams = new byte[optionLen];
+                            packet.get(expectedParams);
+                            expectedLen = optionLen;
+                            break;
+                        case DHCP_MESSAGE:
+                            expectedLen = optionLen;
+                            message = readAsciiString(packet, optionLen);
+                            break;
+                        case DHCP_MAX_MESSAGE_SIZE:
+                            expectedLen = 2;
+                            maxMessageSize = Short.valueOf(packet.getShort());
+                            break;
+                        case DHCP_RENEWAL_TIME:
+                            expectedLen = 4;
+                            T1 = Integer.valueOf(packet.getInt());
+                            break;
+                        case DHCP_REBINDING_TIME:
+                            expectedLen = 4;
+                            T2 = Integer.valueOf(packet.getInt());
+                            break;
+                        case DHCP_VENDOR_CLASS_ID:
+                            expectedLen = optionLen;
+                            vendorId = readAsciiString(packet, optionLen);
+                            break;
+                        case DHCP_CLIENT_IDENTIFIER: { // Client identifier
+                            byte[] id = new byte[optionLen];
+                            packet.get(id);
+                            expectedLen = optionLen;
+                        } break;
+                        default:
+                            // ignore any other parameters
+                            for (int i = 0; i < optionLen; i++) {
+                                expectedLen++;
+                                byte throwaway = packet.get();
+                            }
+                    }
+
+                    if (expectedLen != optionLen) {
+                        return null;
+                    }
                 }
+            } catch (BufferUnderflowException e) {
+                return null;
             }
         }