Committed by
Gerrit Code Review
[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
Showing
10 changed files
with
105 additions
and
21 deletions
| ... | @@ -263,11 +263,13 @@ public class NeighborAdvertisement extends BasePacket { | ... | @@ -263,11 +263,13 @@ public class NeighborAdvertisement extends BasePacket { |
| 263 | neighborAdvertisement.overrideFlag = (byte) (iscratch >> 29 & 0x1); | 263 | neighborAdvertisement.overrideFlag = (byte) (iscratch >> 29 & 0x1); |
| 264 | bb.get(neighborAdvertisement.targetAddress, 0, Ip6Address.BYTE_LENGTH); | 264 | bb.get(neighborAdvertisement.targetAddress, 0, Ip6Address.BYTE_LENGTH); |
| 265 | 265 | ||
| 266 | - NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() | 266 | + if (bb.limit() - bb.position() > 0) { |
| 267 | - .deserialize(data, bb.position(), bb.limit() - bb.position()); | 267 | + NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() |
| 268 | + .deserialize(data, bb.position(), bb.limit() - bb.position()); | ||
| 268 | 269 | ||
| 269 | - for (NeighborDiscoveryOptions.Option option : options.options()) { | 270 | + for (NeighborDiscoveryOptions.Option option : options.options()) { |
| 270 | - neighborAdvertisement.addOption(option.type(), option.data()); | 271 | + neighborAdvertisement.addOption(option.type(), option.data()); |
| 272 | + } | ||
| 271 | } | 273 | } |
| 272 | 274 | ||
| 273 | return neighborAdvertisement; | 275 | return neighborAdvertisement; | ... | ... |
| ... | @@ -177,11 +177,13 @@ public class NeighborSolicitation extends BasePacket { | ... | @@ -177,11 +177,13 @@ public class NeighborSolicitation extends BasePacket { |
| 177 | bb.getInt(); | 177 | bb.getInt(); |
| 178 | bb.get(neighborSolicitation.targetAddress, 0, Ip6Address.BYTE_LENGTH); | 178 | bb.get(neighborSolicitation.targetAddress, 0, Ip6Address.BYTE_LENGTH); |
| 179 | 179 | ||
| 180 | - NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() | 180 | + if (bb.limit() - bb.position() > 0) { |
| 181 | - .deserialize(data, bb.position(), bb.limit() - bb.position()); | 181 | + NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() |
| 182 | + .deserialize(data, bb.position(), bb.limit() - bb.position()); | ||
| 182 | 183 | ||
| 183 | - for (NeighborDiscoveryOptions.Option option : options.options()) { | 184 | + for (NeighborDiscoveryOptions.Option option : options.options()) { |
| 184 | - neighborSolicitation.addOption(option.type(), option.data()); | 185 | + neighborSolicitation.addOption(option.type(), option.data()); |
| 186 | + } | ||
| 185 | } | 187 | } |
| 186 | 188 | ||
| 187 | return neighborSolicitation; | 189 | return neighborSolicitation; | ... | ... |
| ... | @@ -210,11 +210,13 @@ public class Redirect extends BasePacket { | ... | @@ -210,11 +210,13 @@ public class Redirect extends BasePacket { |
| 210 | bb.get(redirect.targetAddress, 0, Ip6Address.BYTE_LENGTH); | 210 | bb.get(redirect.targetAddress, 0, Ip6Address.BYTE_LENGTH); |
| 211 | bb.get(redirect.destinationAddress, 0, Ip6Address.BYTE_LENGTH); | 211 | bb.get(redirect.destinationAddress, 0, Ip6Address.BYTE_LENGTH); |
| 212 | 212 | ||
| 213 | - NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() | 213 | + if (bb.limit() - bb.position() > 0) { |
| 214 | - .deserialize(data, bb.position(), bb.limit() - bb.position()); | 214 | + NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() |
| 215 | + .deserialize(data, bb.position(), bb.limit() - bb.position()); | ||
| 215 | 216 | ||
| 216 | - for (NeighborDiscoveryOptions.Option option : options.options()) { | 217 | + for (NeighborDiscoveryOptions.Option option : options.options()) { |
| 217 | - redirect.addOption(option.type(), option.data()); | 218 | + redirect.addOption(option.type(), option.data()); |
| 219 | + } | ||
| 218 | } | 220 | } |
| 219 | 221 | ||
| 220 | return redirect; | 222 | return redirect; | ... | ... |
| ... | @@ -310,11 +310,13 @@ public class RouterAdvertisement extends BasePacket { | ... | @@ -310,11 +310,13 @@ public class RouterAdvertisement extends BasePacket { |
| 310 | routerAdvertisement.reachableTime = bb.getInt(); | 310 | routerAdvertisement.reachableTime = bb.getInt(); |
| 311 | routerAdvertisement.retransmitTimer = bb.getInt(); | 311 | routerAdvertisement.retransmitTimer = bb.getInt(); |
| 312 | 312 | ||
| 313 | - NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() | 313 | + if (bb.limit() - bb.position() > 0) { |
| 314 | - .deserialize(data, bb.position(), bb.limit() - bb.position()); | 314 | + NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() |
| 315 | + .deserialize(data, bb.position(), bb.limit() - bb.position()); | ||
| 315 | 316 | ||
| 316 | - for (NeighborDiscoveryOptions.Option option : options.options()) { | 317 | + for (NeighborDiscoveryOptions.Option option : options.options()) { |
| 317 | - routerAdvertisement.addOption(option.type(), option.data()); | 318 | + routerAdvertisement.addOption(option.type(), option.data()); |
| 319 | + } | ||
| 318 | } | 320 | } |
| 319 | 321 | ||
| 320 | return routerAdvertisement; | 322 | return routerAdvertisement; | ... | ... |
| ... | @@ -140,11 +140,13 @@ public class RouterSolicitation extends BasePacket { | ... | @@ -140,11 +140,13 @@ public class RouterSolicitation extends BasePacket { |
| 140 | 140 | ||
| 141 | bb.getInt(); | 141 | bb.getInt(); |
| 142 | 142 | ||
| 143 | - NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() | 143 | + if (bb.limit() - bb.position() > 0) { |
| 144 | - .deserialize(data, bb.position(), bb.limit() - bb.position()); | 144 | + NeighborDiscoveryOptions options = NeighborDiscoveryOptions.deserializer() |
| 145 | + .deserialize(data, bb.position(), bb.limit() - bb.position()); | ||
| 145 | 146 | ||
| 146 | - for (NeighborDiscoveryOptions.Option option : options.options()) { | 147 | + for (NeighborDiscoveryOptions.Option option : options.options()) { |
| 147 | - routerSolicitation.addOption(option.type(), option.data()); | 148 | + routerSolicitation.addOption(option.type(), option.data()); |
| 149 | + } | ||
| 148 | } | 150 | } |
| 149 | 151 | ||
| 150 | return routerSolicitation; | 152 | return routerSolicitation; | ... | ... |
| ... | @@ -20,6 +20,9 @@ import org.junit.Test; | ... | @@ -20,6 +20,9 @@ import org.junit.Test; |
| 20 | import org.onlab.packet.DeserializationException; | 20 | import org.onlab.packet.DeserializationException; |
| 21 | import org.onlab.packet.Deserializer; | 21 | import org.onlab.packet.Deserializer; |
| 22 | import org.onlab.packet.MacAddress; | 22 | import org.onlab.packet.MacAddress; |
| 23 | +import org.onlab.packet.PacketTestUtils; | ||
| 24 | + | ||
| 25 | +import java.nio.ByteBuffer; | ||
| 23 | 26 | ||
| 24 | import static org.hamcrest.Matchers.is; | 27 | import static org.hamcrest.Matchers.is; |
| 25 | import static org.junit.Assert.assertArrayEquals; | 28 | import static org.junit.Assert.assertArrayEquals; |
| ... | @@ -76,6 +79,20 @@ public class NeighborAdvertisementTest { | ... | @@ -76,6 +79,20 @@ public class NeighborAdvertisementTest { |
| 76 | assertArrayEquals(na.serialize(), bytePacket); | 79 | assertArrayEquals(na.serialize(), bytePacket); |
| 77 | } | 80 | } |
| 78 | 81 | ||
| 82 | + @Test | ||
| 83 | + public void testDeserializeBadInput() throws Exception { | ||
| 84 | + PacketTestUtils.testDeserializeBadInput(NeighborAdvertisement.deserializer()); | ||
| 85 | + } | ||
| 86 | + | ||
| 87 | + @Test | ||
| 88 | + public void testDeserializeTruncated() throws Exception { | ||
| 89 | + // Run the truncation test only on the NeighborAdvertisement header | ||
| 90 | + byte[] naHeader = new byte[NeighborAdvertisement.HEADER_LENGTH]; | ||
| 91 | + ByteBuffer.wrap(bytePacket).get(naHeader); | ||
| 92 | + | ||
| 93 | + PacketTestUtils.testDeserializeTruncated(NeighborAdvertisement.deserializer(), naHeader); | ||
| 94 | + } | ||
| 95 | + | ||
| 79 | /** | 96 | /** |
| 80 | * Tests deserialize and getters. | 97 | * Tests deserialize and getters. |
| 81 | */ | 98 | */ | ... | ... |
| ... | @@ -20,6 +20,9 @@ import org.junit.Test; | ... | @@ -20,6 +20,9 @@ import org.junit.Test; |
| 20 | import org.onlab.packet.DeserializationException; | 20 | import org.onlab.packet.DeserializationException; |
| 21 | import org.onlab.packet.Deserializer; | 21 | import org.onlab.packet.Deserializer; |
| 22 | import org.onlab.packet.MacAddress; | 22 | import org.onlab.packet.MacAddress; |
| 23 | +import org.onlab.packet.PacketTestUtils; | ||
| 24 | + | ||
| 25 | +import java.nio.ByteBuffer; | ||
| 23 | 26 | ||
| 24 | import static org.hamcrest.Matchers.is; | 27 | import static org.hamcrest.Matchers.is; |
| 25 | import static org.junit.Assert.assertArrayEquals; | 28 | import static org.junit.Assert.assertArrayEquals; |
| ... | @@ -79,6 +82,20 @@ public class NeighborSolicitationTest { | ... | @@ -79,6 +82,20 @@ public class NeighborSolicitationTest { |
| 79 | assertArrayEquals(ns.serialize(), bytePacket); | 82 | assertArrayEquals(ns.serialize(), bytePacket); |
| 80 | } | 83 | } |
| 81 | 84 | ||
| 85 | + @Test | ||
| 86 | + public void testDeserializeBadInput() throws Exception { | ||
| 87 | + PacketTestUtils.testDeserializeBadInput(NeighborSolicitation.deserializer()); | ||
| 88 | + } | ||
| 89 | + | ||
| 90 | + @Test | ||
| 91 | + public void testDeserializeTruncated() throws Exception { | ||
| 92 | + // Run the truncation test only on the NeighborSolicitation header | ||
| 93 | + byte[] nsHeader = new byte[NeighborSolicitation.HEADER_LENGTH]; | ||
| 94 | + ByteBuffer.wrap(bytePacket).get(nsHeader); | ||
| 95 | + | ||
| 96 | + PacketTestUtils.testDeserializeTruncated(NeighborSolicitation.deserializer(), nsHeader); | ||
| 97 | + } | ||
| 98 | + | ||
| 82 | /** | 99 | /** |
| 83 | * Tests deserialize and getters. | 100 | * Tests deserialize and getters. |
| 84 | */ | 101 | */ | ... | ... |
| ... | @@ -20,6 +20,9 @@ import org.junit.Test; | ... | @@ -20,6 +20,9 @@ import org.junit.Test; |
| 20 | import org.onlab.packet.DeserializationException; | 20 | import org.onlab.packet.DeserializationException; |
| 21 | import org.onlab.packet.Deserializer; | 21 | import org.onlab.packet.Deserializer; |
| 22 | import org.onlab.packet.MacAddress; | 22 | import org.onlab.packet.MacAddress; |
| 23 | +import org.onlab.packet.PacketTestUtils; | ||
| 24 | + | ||
| 25 | +import java.nio.ByteBuffer; | ||
| 23 | 26 | ||
| 24 | import static org.hamcrest.Matchers.is; | 27 | import static org.hamcrest.Matchers.is; |
| 25 | import static org.junit.Assert.assertArrayEquals; | 28 | import static org.junit.Assert.assertArrayEquals; |
| ... | @@ -89,6 +92,20 @@ public class RedirectTest { | ... | @@ -89,6 +92,20 @@ public class RedirectTest { |
| 89 | assertArrayEquals(rd.serialize(), bytePacket); | 92 | assertArrayEquals(rd.serialize(), bytePacket); |
| 90 | } | 93 | } |
| 91 | 94 | ||
| 95 | + @Test | ||
| 96 | + public void testDeserializeBadInput() throws Exception { | ||
| 97 | + PacketTestUtils.testDeserializeBadInput(Redirect.deserializer()); | ||
| 98 | + } | ||
| 99 | + | ||
| 100 | + @Test | ||
| 101 | + public void testDeserializeTruncated() throws Exception { | ||
| 102 | + // Run the truncation test only on the Redirect header | ||
| 103 | + byte[] rdHeader = new byte[Redirect.HEADER_LENGTH]; | ||
| 104 | + ByteBuffer.wrap(bytePacket).get(rdHeader); | ||
| 105 | + | ||
| 106 | + PacketTestUtils.testDeserializeTruncated(Redirect.deserializer(), rdHeader); | ||
| 107 | + } | ||
| 108 | + | ||
| 92 | /** | 109 | /** |
| 93 | * Tests deserialize and getters. | 110 | * Tests deserialize and getters. |
| 94 | */ | 111 | */ | ... | ... |
| ... | @@ -20,6 +20,9 @@ import org.junit.Test; | ... | @@ -20,6 +20,9 @@ import org.junit.Test; |
| 20 | import org.onlab.packet.DeserializationException; | 20 | import org.onlab.packet.DeserializationException; |
| 21 | import org.onlab.packet.Deserializer; | 21 | import org.onlab.packet.Deserializer; |
| 22 | import org.onlab.packet.MacAddress; | 22 | import org.onlab.packet.MacAddress; |
| 23 | +import org.onlab.packet.PacketTestUtils; | ||
| 24 | + | ||
| 25 | +import java.nio.ByteBuffer; | ||
| 23 | 26 | ||
| 24 | import static org.hamcrest.Matchers.is; | 27 | import static org.hamcrest.Matchers.is; |
| 25 | import static org.junit.Assert.assertArrayEquals; | 28 | import static org.junit.Assert.assertArrayEquals; |
| ... | @@ -70,6 +73,20 @@ public class RouterAdvertisementTest { | ... | @@ -70,6 +73,20 @@ public class RouterAdvertisementTest { |
| 70 | assertArrayEquals(ra.serialize(), bytePacket); | 73 | assertArrayEquals(ra.serialize(), bytePacket); |
| 71 | } | 74 | } |
| 72 | 75 | ||
| 76 | + @Test | ||
| 77 | + public void testDeserializeBadInput() throws Exception { | ||
| 78 | + PacketTestUtils.testDeserializeBadInput(RouterAdvertisement.deserializer()); | ||
| 79 | + } | ||
| 80 | + | ||
| 81 | + @Test | ||
| 82 | + public void testDeserializeTruncated() throws Exception { | ||
| 83 | + // Run the truncation test only on the RouterAdvertisement header | ||
| 84 | + byte[] raHeader = new byte[RouterAdvertisement.HEADER_LENGTH]; | ||
| 85 | + ByteBuffer.wrap(bytePacket).get(raHeader); | ||
| 86 | + | ||
| 87 | + PacketTestUtils.testDeserializeTruncated(RouterAdvertisement.deserializer(), raHeader); | ||
| 88 | + } | ||
| 89 | + | ||
| 73 | /** | 90 | /** |
| 74 | * Tests deserialize and getters. | 91 | * Tests deserialize and getters. |
| 75 | */ | 92 | */ | ... | ... |
| ... | @@ -21,6 +21,8 @@ import org.onlab.packet.Deserializer; | ... | @@ -21,6 +21,8 @@ import org.onlab.packet.Deserializer; |
| 21 | import org.onlab.packet.MacAddress; | 21 | import org.onlab.packet.MacAddress; |
| 22 | import org.onlab.packet.PacketTestUtils; | 22 | import org.onlab.packet.PacketTestUtils; |
| 23 | 23 | ||
| 24 | +import java.nio.ByteBuffer; | ||
| 25 | + | ||
| 24 | import static org.hamcrest.Matchers.is; | 26 | import static org.hamcrest.Matchers.is; |
| 25 | import static org.junit.Assert.assertArrayEquals; | 27 | import static org.junit.Assert.assertArrayEquals; |
| 26 | import static org.junit.Assert.assertFalse; | 28 | import static org.junit.Assert.assertFalse; |
| ... | @@ -71,7 +73,11 @@ public class RouterSolicitationTest { | ... | @@ -71,7 +73,11 @@ public class RouterSolicitationTest { |
| 71 | 73 | ||
| 72 | @Test | 74 | @Test |
| 73 | public void testDeserializeTruncated() throws Exception { | 75 | public void testDeserializeTruncated() throws Exception { |
| 74 | - PacketTestUtils.testDeserializeTruncated(RouterSolicitation.deserializer(), bytePacket); | 76 | + // Run the truncation test only on the RouterSolicitation header |
| 77 | + byte[] rsHeader = new byte[RouterSolicitation.HEADER_LENGTH]; | ||
| 78 | + ByteBuffer.wrap(bytePacket).get(rsHeader); | ||
| 79 | + | ||
| 80 | + PacketTestUtils.testDeserializeTruncated(RouterSolicitation.deserializer(), rsHeader); | ||
| 75 | } | 81 | } |
| 76 | 82 | ||
| 77 | /** | 83 | /** | ... | ... |
-
Please register or login to post a comment