Merge pull request #570 from cniles/master

Fix integer addition overflow.
drewnoakes · Mar 31, 2022 · 85ec243 · 85ec243
2 parents e9733f0 + ecff9b4
commit 85ec243
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 11 deletions.
diff --git a/Source/com/drew/lang/SequentialByteArrayReader.java b/Source/com/drew/lang/SequentialByteArrayReader.java
@@ -70,7 +70,7 @@ public byte getByte() throws IOException
     @Override
     public byte[] getBytes(int count) throws IOException
     {
-        if (_index + count > _bytes.length) {
+        if ((long)_index + count > _bytes.length) {
             throw new EOFException("End of data reached.");
         }
 
@@ -84,7 +84,7 @@ public byte[] getBytes(int count) throws IOException
     @Override
     public void getBytes(@NotNull byte[] buffer, int offset, int count) throws IOException
     {
-        if (_index + count > _bytes.length) {
+        if ((long)_index + count > _bytes.length) {
             throw new EOFException("End of data reached.");
         }
 
@@ -113,13 +113,13 @@ public boolean trySkip(long n) throws IOException
             throw new IllegalArgumentException("n must be zero or greater.");
         }
 
-        _index += n;
-
-        if (_index > _bytes.length) {
+        if (_index + n > _bytes.length)  {
             _index = _bytes.length;
             return false;
         }
 
+        _index += n;
+
         return true;
     }
 

diff --git a/Source/com/drew/lang/StreamReader.java b/Source/com/drew/lang/StreamReader.java
@@ -68,9 +68,14 @@ public byte getByte() throws IOException
     @Override
     public byte[] getBytes(int count) throws IOException
     {
-        byte[] bytes = new byte[count];
-        getBytes(bytes, 0, count);
-        return bytes;
+        try {
+            byte[] bytes = new byte[count];
+            getBytes(bytes, 0, count);
+            return bytes;
+        } catch (OutOfMemoryError e) {
+            throw new EOFException("End of data reached.");
+        }
+
     }
 
     @Override

diff --git a/Tests/com/drew/lang/SequentialAccessTestBase.java b/Tests/com/drew/lang/SequentialAccessTestBase.java
@@ -40,7 +40,7 @@ public abstract class SequentialAccessTestBase
     @Test
     public void testDefaultEndianness()
     {
-        assertEquals(true, createReader(new byte[1]).isMotorolaByteOrder());
+        assertTrue(createReader(new byte[1]).isMotorolaByteOrder());
     }
 
     @Test
@@ -269,13 +269,20 @@ public void testGetBytes() throws IOException
     @Test
     public void testOverflowBoundsCalculation()
     {
-        SequentialReader reader = createReader(new byte[10]);
-
         try {
+            SequentialReader reader = createReader(new byte[10]);
             reader.getBytes(15);
         } catch (IOException e) {
             assertEquals("End of data reached.", e.getMessage());
         }
+
+        try {
+            SequentialReader reader = createReader(new byte[10]);
+            reader.getBytes(5);
+            reader.getBytes(Integer.MAX_VALUE);
+        } catch (IOException e) {
+            assertEquals("End of data reached.", e.getMessage());
+        }
     }
 
     @Test
@@ -325,6 +332,13 @@ public void testSkipEOF() throws Exception
             reader.skip(1);
             fail("Expecting exception");
         } catch (EOFException ignored) {}
+
+        try {
+            reader = createReader(new byte[100]);
+            reader.skip(50);
+            reader.skip(Integer.MAX_VALUE);
+            fail("Expecting exception");
+        } catch (EOFException ignored) {}
     }
 
     @Test
@@ -336,5 +350,9 @@ public void testTrySkipEOF() throws Exception
         assertTrue(reader.trySkip(1));
         assertTrue(reader.trySkip(1));
         assertFalse(reader.trySkip(1));
+
+        reader = createReader(new byte[100]);
+        reader.getBytes(50);
+        assertFalse(reader.trySkip(Integer.MAX_VALUE));
     }
 }