Adding checks to readHeader for invalid firstOffset and lastOffset va…

…lues (#646) * Adding checks to readHeader for invalid values in the header for firstOffset and lastOffset. This will resolve an `ArrayIndexOutOfBoundsException` (#586) crash in Android 9 devices as IOExceptions are handled by `SegmentIntegration.createQueueFile`. * Fixing formatting violations Co-authored-by: Coby Plain <cplain@.atlassian.com>
segmentio · Mar 15, 2020 · f0d6d37 · f0d6d37
1 parent 6a24674
commit f0d6d37
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 3 deletions.
diff --git a/analytics/src/main/java/com/segment/analytics/QueueFile.java b/analytics/src/main/java/com/segment/analytics/QueueFile.java
@@ -154,16 +154,23 @@ private void readHeader() throws IOException {
     raf.seek(0);
     raf.readFully(buffer);
     fileLength = readInt(buffer, 0);
+    elementCount = readInt(buffer, 4);
+    int firstOffset = readInt(buffer, 8);
+    int lastOffset = readInt(buffer, 12);
+
     if (fileLength > raf.length()) {
       throw new IOException(
           "File is truncated. Expected length: " + fileLength + ", Actual length: " + raf.length());
     } else if (fileLength <= 0) {
       throw new IOException(
           "File is corrupt; length stored in header (" + fileLength + ") is invalid.");
+    } else if (firstOffset < 0 || fileLength <= wrapPosition(firstOffset)) {
+      throw new IOException(
+          "File is corrupt; first position stored in header (" + firstOffset + ") is invalid.");
+    } else if (lastOffset < 0 || fileLength <= wrapPosition(lastOffset)) {
+      throw new IOException(
+          "File is corrupt; last position stored in header (" + lastOffset + ") is invalid.");
     }
-    elementCount = readInt(buffer, 4);
-    int firstOffset = readInt(buffer, 8);
-    int lastOffset = readInt(buffer, 12);
     first = readElement(firstOffset);
     last = readElement(lastOffset);
   }

diff --git a/analytics/src/test/java/com/segment/analytics/QueueFileTest.java b/analytics/src/test/java/com/segment/analytics/QueueFileTest.java
@@ -186,6 +186,86 @@ public void testNegativeSizeInHeaderThrows() throws IOException {
     }
   }
 
+  @Test
+  public void testInvalidFirstPositionThrows() throws IOException {
+    RandomAccessFile emptyFile = new RandomAccessFile(file, "rwd");
+    emptyFile.seek(0);
+    emptyFile.writeInt(4096);
+    emptyFile.setLength(4096);
+    emptyFile.seek(8);
+    emptyFile.writeInt(10000);
+    emptyFile.getChannel().force(true);
+    emptyFile.close();
+
+    try {
+      new QueueFile(file);
+      fail("Should have thrown about bad first position value");
+    } catch (IOException ex) {
+      assertThat(ex) //
+          .hasMessage("File is corrupt; first position stored in header (10000) is invalid.");
+    }
+  }
+
+  @Test
+  public void testNegativeFirstPositionThrows() throws IOException {
+    RandomAccessFile emptyFile = new RandomAccessFile(file, "rwd");
+    emptyFile.seek(0);
+    emptyFile.writeInt(4096);
+    emptyFile.setLength(4096);
+    emptyFile.seek(8);
+    emptyFile.writeInt(-2147483648);
+    emptyFile.getChannel().force(true);
+    emptyFile.close();
+
+    try {
+      new QueueFile(file);
+      fail("Should have thrown about bad first position value");
+    } catch (IOException ex) {
+      assertThat(ex) //
+          .hasMessage("File is corrupt; first position stored in header (-2147483648) is invalid.");
+    }
+  }
+
+  @Test
+  public void testInvalidLastPositionThrows() throws IOException {
+    RandomAccessFile emptyFile = new RandomAccessFile(file, "rwd");
+    emptyFile.seek(0);
+    emptyFile.writeInt(4096);
+    emptyFile.setLength(4096);
+    emptyFile.seek(12);
+    emptyFile.writeInt(10000);
+    emptyFile.getChannel().force(true);
+    emptyFile.close();
+
+    try {
+      new QueueFile(file);
+      fail("Should have thrown about bad last position value");
+    } catch (IOException ex) {
+      assertThat(ex) //
+          .hasMessage("File is corrupt; last position stored in header (10000) is invalid.");
+    }
+  }
+
+  @Test
+  public void testNegativeLastPositionThrows() throws IOException {
+    RandomAccessFile emptyFile = new RandomAccessFile(file, "rwd");
+    emptyFile.seek(0);
+    emptyFile.writeInt(4096);
+    emptyFile.setLength(4096);
+    emptyFile.seek(12);
+    emptyFile.writeInt(-2147483648);
+    emptyFile.getChannel().force(true);
+    emptyFile.close();
+
+    try {
+      new QueueFile(file);
+      fail("Should have thrown about bad last position value");
+    } catch (IOException ex) {
+      assertThat(ex) //
+          .hasMessage("File is corrupt; last position stored in header (-2147483648) is invalid.");
+    }
+  }
+
   @Test
   public void removeMultipleDoesNotCorrupt() throws IOException {
     QueueFile queue = new QueueFile(file);