Reject DHCP packets with no magic cookie
This patch adds an explicit check in the DHCP packet parser for
rejecting packets without a magic cookie, instead of relying on the
top-level try-catch-all in the parser.
This allows to add to DHCP error metrics this specific error.
It also allows to add two poor man's fuzzing tests that tries to find
additional gaps in the DHCP packet parser by
- trying to parse all subslices of a valid offer packet.
- trying to parse random byte arrays.
Test: covered by previously introduced malformed DHCP packet unit tests
+ additional fuzzing tests.
Bug: 31850211
Change-Id: If53c9ba9df78d7604ec018c9d67c237ae59c4833
(cherry picked from commit 006e0613016c1a0e0627f992f5a93a7b7198edba)
diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
index b4e9db77..bc8baa1 100644
--- a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
+++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
@@ -27,6 +27,7 @@
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Random;
import junit.framework.TestCase;
import static android.net.dhcp.DhcpPacket.*;
@@ -430,7 +431,7 @@
try {
DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, packet.length, ENCAP_L3);
} catch (DhcpPacket.ParseException expected) {
- assertDhcpErrorCodes(DhcpErrorEvent.PARSING_ERROR, expected.errorCode);
+ assertDhcpErrorCodes(DhcpErrorEvent.DHCP_NO_COOKIE, expected.errorCode);
return;
}
fail("Dhcp packet parsing should have failed");
@@ -472,6 +473,55 @@
assertEquals(Integer.toHexString(expected), Integer.toHexString(got));
}
+ public void testTruncatedOfferPackets() throws Exception {
+ final byte[] packet = HexDump.hexStringToByteArray(
+ // IP header.
+ "450001518d0600004011144dc0a82b01c0a82bf7" +
+ // UDP header.
+ "00430044013d9ac7" +
+ // BOOTP header.
+ "02010600dfc23d1f0002000000000000c0a82bf7c0a82b0100000000" +
+ // MAC address.
+ "30766ff2a90c00000000000000000000" +
+ // Server name.
+ "0000000000000000000000000000000000000000000000000000000000000000" +
+ "0000000000000000000000000000000000000000000000000000000000000000" +
+ // File.
+ "0000000000000000000000000000000000000000000000000000000000000000" +
+ "0000000000000000000000000000000000000000000000000000000000000000" +
+ "0000000000000000000000000000000000000000000000000000000000000000" +
+ "0000000000000000000000000000000000000000000000000000000000000000" +
+ // Options
+ "638253633501023604c0a82b01330400000e103a04000007083b0400000c4e0104ffffff00" +
+ "1c04c0a82bff0304c0a82b010604c0a82b012b0f414e44524f49445f4d455445524544ff");
+
+ for (int len = 0; len < packet.length; len++) {
+ try {
+ DhcpPacket.decodeFullPacket(packet, len, ENCAP_L3);
+ } catch (ParseException e) {
+ if (e.errorCode == DhcpErrorEvent.PARSING_ERROR) {
+ fail(String.format("bad truncated packet of length %d", len));
+ }
+ }
+ }
+ }
+
+ public void testRandomPackets() throws Exception {
+ final int maxRandomPacketSize = 512;
+ final Random r = new Random();
+ for (int i = 0; i < 10000; i++) {
+ byte[] packet = new byte[r.nextInt(maxRandomPacketSize + 1)];
+ r.nextBytes(packet);
+ try {
+ DhcpPacket.decodeFullPacket(packet, packet.length, ENCAP_L3);
+ } catch (ParseException e) {
+ if (e.errorCode == DhcpErrorEvent.PARSING_ERROR) {
+ fail("bad packet: " + HexDump.toHexString(packet));
+ }
+ }
+ }
+ }
+
private byte[] mtuBytes(int mtu) {
// 0x1a02: option 26, length 2. 0xff: no more options.
if (mtu > Short.MAX_VALUE - Short.MIN_VALUE) {
