[ONOS-2808] Properly deserialzes NDP packets without options
In addition, add following cases into NDP unit tests:
- testDeserializeBadInput
- testDeserializeTruncated (NDP headers only, options skipped)
Change-Id: Ia295a5bd7fcdcc25ac556da7bc2eaab13ad8e3b8
diff --git a/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborAdvertisement.java b/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborAdvertisement.java
index 08c749a..99fa0dd 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborAdvertisement.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborAdvertisement.java
@@ -263,11 +263,13 @@
neighborAdvertisement.overrideFlag = (byte) (iscratch >> 29 & 0x1);
bb.get(neighborAdvertisement.targetAddress, 0, Ip6Address.BYTE_LENGTH);
- NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
- .deserialize(data, bb.position(), bb.limit() - bb.position());
+ if (bb.limit() - bb.position() > 0) {
+ NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
+ .deserialize(data, bb.position(), bb.limit() - bb.position());
- for (NeighborDiscoveryOptions.Option option : options.options()) {
- neighborAdvertisement.addOption(option.type(), option.data());
+ for (NeighborDiscoveryOptions.Option option : options.options()) {
+ neighborAdvertisement.addOption(option.type(), option.data());
+ }
}
return neighborAdvertisement;
diff --git a/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborSolicitation.java b/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborSolicitation.java
index 33a6eb2b6..77c119a 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborSolicitation.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ndp/NeighborSolicitation.java
@@ -177,11 +177,13 @@
bb.getInt();
bb.get(neighborSolicitation.targetAddress, 0, Ip6Address.BYTE_LENGTH);
- NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
- .deserialize(data, bb.position(), bb.limit() - bb.position());
+ if (bb.limit() - bb.position() > 0) {
+ NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
+ .deserialize(data, bb.position(), bb.limit() - bb.position());
- for (NeighborDiscoveryOptions.Option option : options.options()) {
- neighborSolicitation.addOption(option.type(), option.data());
+ for (NeighborDiscoveryOptions.Option option : options.options()) {
+ neighborSolicitation.addOption(option.type(), option.data());
+ }
}
return neighborSolicitation;
diff --git a/utils/misc/src/main/java/org/onlab/packet/ndp/Redirect.java b/utils/misc/src/main/java/org/onlab/packet/ndp/Redirect.java
index c3f46dd..51256d4 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ndp/Redirect.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ndp/Redirect.java
@@ -210,11 +210,13 @@
bb.get(redirect.targetAddress, 0, Ip6Address.BYTE_LENGTH);
bb.get(redirect.destinationAddress, 0, Ip6Address.BYTE_LENGTH);
- NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
- .deserialize(data, bb.position(), bb.limit() - bb.position());
+ if (bb.limit() - bb.position() > 0) {
+ NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
+ .deserialize(data, bb.position(), bb.limit() - bb.position());
- for (NeighborDiscoveryOptions.Option option : options.options()) {
- redirect.addOption(option.type(), option.data());
+ for (NeighborDiscoveryOptions.Option option : options.options()) {
+ redirect.addOption(option.type(), option.data());
+ }
}
return redirect;
diff --git a/utils/misc/src/main/java/org/onlab/packet/ndp/RouterAdvertisement.java b/utils/misc/src/main/java/org/onlab/packet/ndp/RouterAdvertisement.java
index e4ef2c6..597fc9f 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ndp/RouterAdvertisement.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ndp/RouterAdvertisement.java
@@ -310,11 +310,13 @@
routerAdvertisement.reachableTime = bb.getInt();
routerAdvertisement.retransmitTimer = bb.getInt();
- NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
- .deserialize(data, bb.position(), bb.limit() - bb.position());
+ if (bb.limit() - bb.position() > 0) {
+ NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
+ .deserialize(data, bb.position(), bb.limit() - bb.position());
- for (NeighborDiscoveryOptions.Option option : options.options()) {
- routerAdvertisement.addOption(option.type(), option.data());
+ for (NeighborDiscoveryOptions.Option option : options.options()) {
+ routerAdvertisement.addOption(option.type(), option.data());
+ }
}
return routerAdvertisement;
diff --git a/utils/misc/src/main/java/org/onlab/packet/ndp/RouterSolicitation.java b/utils/misc/src/main/java/org/onlab/packet/ndp/RouterSolicitation.java
index 07729df..e279a40 100644
--- a/utils/misc/src/main/java/org/onlab/packet/ndp/RouterSolicitation.java
+++ b/utils/misc/src/main/java/org/onlab/packet/ndp/RouterSolicitation.java
@@ -140,11 +140,13 @@
bb.getInt();
- NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
- .deserialize(data, bb.position(), bb.limit() - bb.position());
+ if (bb.limit() - bb.position() > 0) {
+ NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer()
+ .deserialize(data, bb.position(), bb.limit() - bb.position());
- for (NeighborDiscoveryOptions.Option option : options.options()) {
- routerSolicitation.addOption(option.type(), option.data());
+ for (NeighborDiscoveryOptions.Option option : options.options()) {
+ routerSolicitation.addOption(option.type(), option.data());
+ }
}
return routerSolicitation;
diff --git a/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborAdvertisementTest.java b/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborAdvertisementTest.java
index 303db07..d537251 100644
--- a/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborAdvertisementTest.java
+++ b/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborAdvertisementTest.java
@@ -20,6 +20,9 @@
import org.onlab.packet.DeserializationException;
import org.onlab.packet.Deserializer;
import org.onlab.packet.MacAddress;
+import org.onlab.packet.PacketTestUtils;
+
+import java.nio.ByteBuffer;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertArrayEquals;
@@ -76,6 +79,20 @@
assertArrayEquals(na.serialize(), bytePacket);
}
+ @Test
+ public void testDeserializeBadInput() throws Exception {
+ PacketTestUtils.testDeserializeBadInput(NeighborAdvertisement.deserializer());
+ }
+
+ @Test
+ public void testDeserializeTruncated() throws Exception {
+ // Run the truncation test only on the NeighborAdvertisement header
+ byte[] naHeader = new byte[NeighborAdvertisement.HEADER_LENGTH];
+ ByteBuffer.wrap(bytePacket).get(naHeader);
+
+ PacketTestUtils.testDeserializeTruncated(NeighborAdvertisement.deserializer(), naHeader);
+ }
+
/**
* Tests deserialize and getters.
*/
diff --git a/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborSolicitationTest.java b/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborSolicitationTest.java
index 2571b7a..03e57f5e 100644
--- a/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborSolicitationTest.java
+++ b/utils/misc/src/test/java/org/onlab/packet/ndp/NeighborSolicitationTest.java
@@ -20,6 +20,9 @@
import org.onlab.packet.DeserializationException;
import org.onlab.packet.Deserializer;
import org.onlab.packet.MacAddress;
+import org.onlab.packet.PacketTestUtils;
+
+import java.nio.ByteBuffer;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertArrayEquals;
@@ -79,6 +82,20 @@
assertArrayEquals(ns.serialize(), bytePacket);
}
+ @Test
+ public void testDeserializeBadInput() throws Exception {
+ PacketTestUtils.testDeserializeBadInput(NeighborSolicitation.deserializer());
+ }
+
+ @Test
+ public void testDeserializeTruncated() throws Exception {
+ // Run the truncation test only on the NeighborSolicitation header
+ byte[] nsHeader = new byte[NeighborSolicitation.HEADER_LENGTH];
+ ByteBuffer.wrap(bytePacket).get(nsHeader);
+
+ PacketTestUtils.testDeserializeTruncated(NeighborSolicitation.deserializer(), nsHeader);
+ }
+
/**
* Tests deserialize and getters.
*/
diff --git a/utils/misc/src/test/java/org/onlab/packet/ndp/RedirectTest.java b/utils/misc/src/test/java/org/onlab/packet/ndp/RedirectTest.java
index 7d0d56c..3115714 100644
--- a/utils/misc/src/test/java/org/onlab/packet/ndp/RedirectTest.java
+++ b/utils/misc/src/test/java/org/onlab/packet/ndp/RedirectTest.java
@@ -20,6 +20,9 @@
import org.onlab.packet.DeserializationException;
import org.onlab.packet.Deserializer;
import org.onlab.packet.MacAddress;
+import org.onlab.packet.PacketTestUtils;
+
+import java.nio.ByteBuffer;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertArrayEquals;
@@ -89,6 +92,20 @@
assertArrayEquals(rd.serialize(), bytePacket);
}
+ @Test
+ public void testDeserializeBadInput() throws Exception {
+ PacketTestUtils.testDeserializeBadInput(Redirect.deserializer());
+ }
+
+ @Test
+ public void testDeserializeTruncated() throws Exception {
+ // Run the truncation test only on the Redirect header
+ byte[] rdHeader = new byte[Redirect.HEADER_LENGTH];
+ ByteBuffer.wrap(bytePacket).get(rdHeader);
+
+ PacketTestUtils.testDeserializeTruncated(Redirect.deserializer(), rdHeader);
+ }
+
/**
* Tests deserialize and getters.
*/
diff --git a/utils/misc/src/test/java/org/onlab/packet/ndp/RouterAdvertisementTest.java b/utils/misc/src/test/java/org/onlab/packet/ndp/RouterAdvertisementTest.java
index d65dd51..ede1574 100644
--- a/utils/misc/src/test/java/org/onlab/packet/ndp/RouterAdvertisementTest.java
+++ b/utils/misc/src/test/java/org/onlab/packet/ndp/RouterAdvertisementTest.java
@@ -20,6 +20,9 @@
import org.onlab.packet.DeserializationException;
import org.onlab.packet.Deserializer;
import org.onlab.packet.MacAddress;
+import org.onlab.packet.PacketTestUtils;
+
+import java.nio.ByteBuffer;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertArrayEquals;
@@ -70,6 +73,20 @@
assertArrayEquals(ra.serialize(), bytePacket);
}
+ @Test
+ public void testDeserializeBadInput() throws Exception {
+ PacketTestUtils.testDeserializeBadInput(RouterAdvertisement.deserializer());
+ }
+
+ @Test
+ public void testDeserializeTruncated() throws Exception {
+ // Run the truncation test only on the RouterAdvertisement header
+ byte[] raHeader = new byte[RouterAdvertisement.HEADER_LENGTH];
+ ByteBuffer.wrap(bytePacket).get(raHeader);
+
+ PacketTestUtils.testDeserializeTruncated(RouterAdvertisement.deserializer(), raHeader);
+ }
+
/**
* Tests deserialize and getters.
*/
diff --git a/utils/misc/src/test/java/org/onlab/packet/ndp/RouterSolicitationTest.java b/utils/misc/src/test/java/org/onlab/packet/ndp/RouterSolicitationTest.java
index 173baaa..e3b5686 100644
--- a/utils/misc/src/test/java/org/onlab/packet/ndp/RouterSolicitationTest.java
+++ b/utils/misc/src/test/java/org/onlab/packet/ndp/RouterSolicitationTest.java
@@ -21,6 +21,8 @@
import org.onlab.packet.MacAddress;
import org.onlab.packet.PacketTestUtils;
+import java.nio.ByteBuffer;
+
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertFalse;
@@ -71,7 +73,11 @@
@Test
public void testDeserializeTruncated() throws Exception {
- PacketTestUtils.testDeserializeTruncated(RouterSolicitation.deserializer(), bytePacket);
+ // Run the truncation test only on the RouterSolicitation header
+ byte[] rsHeader = new byte[RouterSolicitation.HEADER_LENGTH];
+ ByteBuffer.wrap(bytePacket).get(rsHeader);
+
+ PacketTestUtils.testDeserializeTruncated(RouterSolicitation.deserializer(), rsHeader);
}
/**