Deal with null characters in string options.
This currently truncates all strings at the first NULL character,
except for vendorInfo, which is an opaque string.
Bug: 19985674
Change-Id: Ie53b2c55eb8a5204d7b2c7e2d8587743d923647a
diff --git a/services/net/java/android/net/dhcp/DhcpPacket.java b/services/net/java/android/net/dhcp/DhcpPacket.java
index a232a6e..d41629d 100644
--- a/services/net/java/android/net/dhcp/DhcpPacket.java
+++ b/services/net/java/android/net/dhcp/DhcpPacket.java
@@ -611,10 +611,22 @@
/**
* Reads a string of specified length from the buffer.
*/
- private static String readAsciiString(ByteBuffer buf, int byteCount) {
+ private static String readAsciiString(ByteBuffer buf, int byteCount, boolean nullOk) {
byte[] bytes = new byte[byteCount];
buf.get(bytes);
- return new String(bytes, 0, bytes.length, StandardCharsets.US_ASCII);
+ int length = bytes.length;
+ if (!nullOk) {
+ // Stop at the first null byte. This is because some DHCP options (e.g., the domain
+ // name) are passed to netd via FrameworkListener, which refuses arguments containing
+ // null bytes. We don't do this by default because vendorInfo is an opaque string which
+ // could in theory contain null bytes.
+ for (length = 0; length < bytes.length; length++) {
+ if (bytes[length] == 0) {
+ break;
+ }
+ }
+ }
+ return new String(bytes, 0, length, StandardCharsets.US_ASCII);
}
/**
@@ -797,7 +809,7 @@
break;
case DHCP_HOST_NAME:
expectedLen = optionLen;
- hostName = readAsciiString(packet, optionLen);
+ hostName = readAsciiString(packet, optionLen, false);
break;
case DHCP_MTU:
expectedLen = 2;
@@ -805,7 +817,7 @@
break;
case DHCP_DOMAIN_NAME:
expectedLen = optionLen;
- domainName = readAsciiString(packet, optionLen);
+ domainName = readAsciiString(packet, optionLen, false);
break;
case DHCP_BROADCAST_ADDRESS:
bcAddr = readIpAddress(packet);
@@ -834,7 +846,7 @@
break;
case DHCP_MESSAGE:
expectedLen = optionLen;
- message = readAsciiString(packet, optionLen);
+ message = readAsciiString(packet, optionLen, false);
break;
case DHCP_MAX_MESSAGE_SIZE:
expectedLen = 2;
@@ -850,7 +862,7 @@
break;
case DHCP_VENDOR_CLASS_ID:
expectedLen = optionLen;
- vendorId = readAsciiString(packet, optionLen);
+ vendorId = readAsciiString(packet, optionLen, true);
break;
case DHCP_CLIENT_IDENTIFIER: { // Client identifier
byte[] id = new byte[optionLen];
diff --git a/services/tests/servicestests/Android.mk b/services/tests/servicestests/Android.mk
index f25fc62..33979b1 100644
--- a/services/tests/servicestests/Android.mk
+++ b/services/tests/servicestests/Android.mk
@@ -10,6 +10,7 @@
LOCAL_STATIC_JAVA_LIBRARIES := \
services.core \
services.devicepolicy \
+ services.net \
easymocklib \
guava \
mockito-target
diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
new file mode 100644
index 0000000..2658937
--- /dev/null
+++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.net.dhcp;
+
+import android.net.NetworkUtils;
+import android.system.OsConstants;
+import android.test.suitebuilder.annotation.SmallTest;
+import junit.framework.TestCase;
+
+import java.net.Inet4Address;
+import java.nio.ByteBuffer;
+
+import static android.net.dhcp.DhcpPacket.*;
+
+
+public class DhcpPacketTest extends TestCase {
+
+ private static Inet4Address SERVER_ADDR =
+ (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");
+ private static Inet4Address CLIENT_ADDR =
+ (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.234");
+ private static byte[] CLIENT_MAC = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+ class TestDhcpPacket extends DhcpPacket {
+ private byte mType;
+ // TODO: Make this a map of option numbers to bytes instead.
+ private byte[] mDomainBytes, mVendorInfoBytes;
+
+ public TestDhcpPacket(byte type, byte[] domainBytes, byte[] vendorInfoBytes) {
+ super(0xdeadbeef, INADDR_ANY, CLIENT_ADDR, INADDR_ANY, INADDR_ANY, CLIENT_MAC, true);
+ mType = type;
+ mDomainBytes = domainBytes;
+ mVendorInfoBytes = vendorInfoBytes;
+ }
+
+ public ByteBuffer buildPacket(int encap, short unusedDestUdp, short unusedSrcUdp) {
+ ByteBuffer result = ByteBuffer.allocate(MAX_LENGTH);
+ fillInPacket(encap, CLIENT_ADDR, SERVER_ADDR,
+ DHCP_CLIENT, DHCP_SERVER, result, DHCP_BOOTREPLY, false);
+ return result;
+ }
+
+ public void finishPacket(ByteBuffer buffer) {
+ addTlv(buffer, DHCP_MESSAGE_TYPE, mType);
+ if (mDomainBytes != null) {
+ addTlv(buffer, DHCP_DOMAIN_NAME, mDomainBytes);
+ }
+ if (mVendorInfoBytes != null) {
+ addTlv(buffer, DHCP_VENDOR_CLASS_ID, mVendorInfoBytes);
+ }
+ addTlvEnd(buffer);
+ }
+
+ // Convenience method.
+ public ByteBuffer build() {
+ // ENCAP_BOOTP packets don't contain ports, so just pass in 0.
+ ByteBuffer pkt = buildPacket(ENCAP_BOOTP, (short) 0, (short) 0);
+ pkt.flip();
+ return pkt;
+ }
+ }
+
+ private void assertDomainAndVendorInfoParses(
+ String expectedDomain, byte[] domainBytes,
+ String expectedVendorInfo, byte[] vendorInfoBytes) {
+ ByteBuffer packet = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER,
+ domainBytes, vendorInfoBytes).build();
+ DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
+ assertEquals(expectedDomain, offerPacket.mDomainName);
+ assertEquals(expectedVendorInfo, offerPacket.mVendorId);
+ }
+
+ @SmallTest
+ public void testDomainName() throws Exception {
+ byte[] nullByte = new byte[] { 0x00 };
+ byte[] twoNullBytes = new byte[] { 0x00, 0x00 };
+ byte[] nonNullDomain = new byte[] {
+ (byte) 'g', (byte) 'o', (byte) 'o', (byte) '.', (byte) 'g', (byte) 'l'
+ };
+ byte[] trailingNullDomain = new byte[] {
+ (byte) 'g', (byte) 'o', (byte) 'o', (byte) '.', (byte) 'g', (byte) 'l', 0x00
+ };
+ byte[] embeddedNullsDomain = new byte[] {
+ (byte) 'g', (byte) 'o', (byte) 'o', 0x00, 0x00, (byte) 'g', (byte) 'l'
+ };
+ byte[] metered = "ANDROID_METERED".getBytes("US-ASCII");
+
+ byte[] meteredEmbeddedNull = metered.clone();
+ meteredEmbeddedNull[7] = (char) 0;
+
+ byte[] meteredTrailingNull = metered.clone();
+ meteredTrailingNull[meteredTrailingNull.length - 1] = (char) 0;
+
+ assertDomainAndVendorInfoParses("", nullByte, "\u0000", nullByte);
+ assertDomainAndVendorInfoParses("", twoNullBytes, "\u0000\u0000", twoNullBytes);
+ assertDomainAndVendorInfoParses("goo.gl", nonNullDomain, "ANDROID_METERED", metered);
+ assertDomainAndVendorInfoParses("goo", embeddedNullsDomain,
+ "ANDROID\u0000METERED", meteredEmbeddedNull);
+ assertDomainAndVendorInfoParses("goo.gl", trailingNullDomain,
+ "ANDROID_METERE\u0000", meteredTrailingNull);
+ }
+}