summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Paul Jensen <pauljensen@google.com> 2016-03-25 11:39:58 -0400
committer Paul Jensen <pauljensen@google.com> 2016-03-29 12:03:34 -0400
commit1c71cb3e728c5f7b3bc76daf581e108ed5c0fa3c (patch)
tree14ec0c953442011b408d170a2f1f89842da2d2aa
parenta2b88f633980422b672a225c6aa96019547f5676 (diff)
Fix potential ApfFilter bugs by careful ByteBuffer use
Avoid adjusting ApfFilter.Ra.mPacket's postion() and limit() in matches(). This avoids potential bugs in other parts of the code that previously relied on limit() being reset. Also for good measure change some limit() calls to capacity() as it's more final. Change-Id: I466e87ce6838f68654b24f2c9543a6cd547d3f87
-rw-r--r--services/net/java/android/net/apf/ApfFilter.java18
1 files changed, 7 insertions, 11 deletions
diff --git a/services/net/java/android/net/apf/ApfFilter.java b/services/net/java/android/net/apf/ApfFilter.java
index ebbf99101e42..3abfb0a15139 100644
--- a/services/net/java/android/net/apf/ApfFilter.java
+++ b/services/net/java/android/net/apf/ApfFilter.java
@@ -225,6 +225,7 @@ public class ApfFilter {
private static final int ICMP6_4_BYTE_LIFETIME_OFFSET = 4;
private static final int ICMP6_4_BYTE_LIFETIME_LEN = 4;
+ // Note: mPacket's position() cannot be assumed to be reset.
private final ByteBuffer mPacket;
// List of binary ranges that include the whole packet except the lifetimes.
// Pairs consist of offset and length.
@@ -378,17 +379,12 @@ public class ApfFilter {
// Ignoring lifetimes (which may change) does {@code packet} match this RA?
boolean matches(byte[] packet, int length) {
- if (length != mPacket.limit()) return false;
- ByteBuffer a = ByteBuffer.wrap(packet);
- ByteBuffer b = mPacket;
+ if (length != mPacket.capacity()) return false;
+ byte[] referencePacket = mPacket.array();
for (Pair<Integer, Integer> nonLifetime : mNonLifetimes) {
- a.clear();
- b.clear();
- a.position(nonLifetime.first);
- b.position(nonLifetime.first);
- a.limit(nonLifetime.first + nonLifetime.second);
- b.limit(nonLifetime.first + nonLifetime.second);
- if (a.compareTo(b) != 0) return false;
+ for (int i = nonLifetime.first; i < (nonLifetime.first + nonLifetime.second); i++) {
+ if (packet[i] != referencePacket[i]) return false;
+ }
}
return true;
}
@@ -440,7 +436,7 @@ public class ApfFilter {
String nextFilterLabel = "Ra" + getUniqueNumberLocked();
// Skip if packet is not the right size
gen.addLoadFromMemory(Register.R0, gen.PACKET_SIZE_MEMORY_SLOT);
- gen.addJumpIfR0NotEquals(mPacket.limit(), nextFilterLabel);
+ gen.addJumpIfR0NotEquals(mPacket.capacity(), nextFilterLabel);
int filterLifetime = (int)(currentLifetime() / FRACTION_OF_LIFETIME_TO_FILTER);
// Skip filter if expired
gen.addLoadFromMemory(Register.R0, gen.FILTER_AGE_MEMORY_SLOT);