Merge "Parse ND options in netlink message as an ByteBuffer slice."
diff --git a/common/device/com/android/net/module/util/netlink/NdOption.java b/common/device/com/android/net/module/util/netlink/NdOption.java
index 6755497..defc88a 100644
--- a/common/device/com/android/net/module/util/netlink/NdOption.java
+++ b/common/device/com/android/net/module/util/netlink/NdOption.java
@@ -16,6 +16,8 @@
package com.android.net.module.util.netlink;
+import androidx.annotation.NonNull;
+
import java.nio.ByteBuffer;
/**
@@ -50,8 +52,8 @@
* @param buf the buffer to parse.
* @return a subclass of {@link NdOption}, or {@code null} for an unknown or malformed option.
*/
- public static NdOption parse(ByteBuffer buf) {
- if (buf == null || buf.remaining() < STRUCT_SIZE) return null;
+ public static NdOption parse(@NonNull ByteBuffer buf) {
+ if (buf.remaining() < STRUCT_SIZE) return null;
// Peek the type without advancing the buffer.
byte type = buf.get(buf.position());
diff --git a/common/device/com/android/net/module/util/netlink/NduseroptMessage.java b/common/device/com/android/net/module/util/netlink/NduseroptMessage.java
index 4e3b9f2..9d2402d 100644
--- a/common/device/com/android/net/module/util/netlink/NduseroptMessage.java
+++ b/common/device/com/android/net/module/util/netlink/NduseroptMessage.java
@@ -19,6 +19,7 @@
import static android.system.OsConstants.AF_INET6;
import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
import java.net.Inet6Address;
import java.net.InetAddress;
@@ -56,6 +57,7 @@
* But if it does, we can simply update this code, since userspace is typically newer than the
* kernel.
*/
+ @Nullable
public final NdOption option;
/** The IP address that sent the packet containing the option. */
@@ -80,22 +82,26 @@
// Ensure we don't read past opts_len even if the option length is invalid.
// Note that this check is not really necessary since if the option length is not valid,
// this struct won't be very useful to the caller.
+ //
+ // It's safer to pass the slice of original ByteBuffer to just parse the ND option field,
+ // although parsing ND option might throw exception or return null, it won't break the
+ // original ByteBuffer position.
buf.order(ByteOrder.BIG_ENDIAN);
- int oldLimit = buf.limit();
- buf.limit(start + STRUCT_SIZE + opts_len);
try {
- option = NdOption.parse(buf);
+ final ByteBuffer slice = buf.slice();
+ slice.limit(opts_len);
+ option = NdOption.parse(slice);
} finally {
- buf.limit(oldLimit);
+ // Advance buffer position according to opts_len in the header. ND option length might
+ // be incorrect in the malformed packet.
+ int newPosition = start + STRUCT_SIZE + opts_len;
+ if (newPosition >= buf.limit()) {
+ throw new IllegalArgumentException("ND option extends past end of buffer");
+ }
+ buf.position(newPosition);
}
- // The source address.
- int newPosition = start + STRUCT_SIZE + opts_len;
- if (newPosition >= buf.limit()) {
- throw new IllegalArgumentException("ND options extend past end of buffer");
- }
- buf.position(newPosition);
-
+ // The source address attribute.
StructNlAttr nla = StructNlAttr.parse(buf);
if (nla == null || nla.nla_type != NDUSEROPT_SRCADDR || nla.nla_value == null) {
throw new IllegalArgumentException("Invalid source address in ND useropt");
diff --git a/common/device/com/android/net/module/util/netlink/StructNdOptPref64.java b/common/device/com/android/net/module/util/netlink/StructNdOptPref64.java
index bde6983..f6b2e0e 100644
--- a/common/device/com/android/net/module/util/netlink/StructNdOptPref64.java
+++ b/common/device/com/android/net/module/util/netlink/StructNdOptPref64.java
@@ -135,7 +135,7 @@
* (for example, if it was truncated, or if the prefix length code was wrong).
*/
public static StructNdOptPref64 parse(@NonNull ByteBuffer buf) {
- if (buf == null || buf.remaining() < STRUCT_SIZE) return null;
+ if (buf.remaining() < STRUCT_SIZE) return null;
try {
return new StructNdOptPref64(buf);
} catch (IllegalArgumentException e) {
diff --git a/common/tests/unit/src/com/android/net/module/util/netlink/NduseroptMessageTest.java b/common/tests/unit/src/com/android/net/module/util/netlink/NduseroptMessageTest.java
index f297108..4fc5ec2 100644
--- a/common/tests/unit/src/com/android/net/module/util/netlink/NduseroptMessageTest.java
+++ b/common/tests/unit/src/com/android/net/module/util/netlink/NduseroptMessageTest.java
@@ -139,6 +139,19 @@
}
@Test
+ public void testParseTruncatedRdnssOptionWithinNetlinkMessage() throws Exception {
+ final String truncatedHexBytes =
+ "38000000440000000000000000000000"
+ + "0A0018001E0000008600000000000000"
+ + "1903000000001770FD123456789000000000000000000001"; // RDNSS option
+
+ ByteBuffer buf = toBuffer(truncatedHexBytes);
+ buf.order(ByteOrder.nativeOrder());
+ NetlinkMessage nlMsg = NetlinkMessage.parse(buf, NETLINK_ROUTE);
+ assertNull(nlMsg);
+ }
+
+ @Test
public void testParseUnknownOptionWithinNetlinkMessage() throws Exception {
final String hexBytes =
"4C000000440000000000000000000000"
diff --git a/common/tests/unit/src/com/android/net/module/util/netlink/StructNdOptRdnssTest.java b/common/tests/unit/src/com/android/net/module/util/netlink/StructNdOptRdnssTest.java
index 7df8fa7..1dcb9b5 100644
--- a/common/tests/unit/src/com/android/net/module/util/netlink/StructNdOptRdnssTest.java
+++ b/common/tests/unit/src/com/android/net/module/util/netlink/StructNdOptRdnssTest.java
@@ -163,11 +163,6 @@
}
@Test
- public void testParsing_nullByteBuffer() {
- assertNull(StructNdOptRdnss.parse(null));
- }
-
- @Test
public void testParsing_invalidByteBufferLength() throws Exception {
final ByteBuffer buf = makeRdnssOption((byte) ICMPV6_ND_OPTION_RDNSS,
(byte) 5 /* length */, 3600 /* lifetime */, DNS_SERVER1, DNS_SERVER2);
