fix(lib): harden Pack7 long decoding

Bound Pack7 long decoding to ten input bytes and fail closed on truncated, over-width, and overflowing encodings. Add boundary, truncation, exact-consumption, and deterministic bounded-read regression tests. Closes original audit finding H5.
2026-07-30 22:33:46 +02:00
parent 2d35e61466
commit 87c59ab7fd
2 changed files with 191 additions and 10 deletions
--- a/lib/src/main/java/zeroecho/core/io/Util.java
+++ b/lib/src/main/java/zeroecho/core/io/Util.java
@@ -81,6 +81,16 @@ public final class Util { // NOPMD
    private static final int DEFAULT_BUFFER_SIZE = 32 * 1024;
    /** Largest unsigned 32-bit value accepted by the packed integer decoder. */
    private static final long MAX_PACKED_INTEGER = 0xffff_ffffL;
+    /** Maximum number of bytes in a packed 64-bit value. */
+    private static final int MAX_PACKED_LONG_BYTES = 10;
+    /** One-based position of the high-order packed payload group. */
+    private static final int PACKED_HIGH_PAYLOAD_POSITION = 1;
+    /** Largest high-order payload permitted in a ten-byte packed long. */
+    private static final int MAX_PACKED_LONG_HIGH_PAYLOAD = 1;
+    /** Bit marking the final byte of a packed value. */
+    private static final int PACKED_TERMINATION_BIT = 0x80;
+    /** Mask selecting the seven payload bits of a packed byte. */
+    private static final int PACKED_PAYLOAD_MASK = 0x7f;

    /**
     * Private constructor to prevent instantiation of this utility class.
@@ -300,22 +310,49 @@ public final class Util { // NOPMD

    /**
     * Reads a long value from the input stream using packed 7-bit encoding
-     * (variable length).
+     * (variable length). Payload groups are stored most-significant first, and the
+     * high bit marks the final byte. At most ten bytes are accepted. In a ten-byte
+     * representation, the first payload group is limited to one bit so that the
+     * encoded value fits exactly in the 64-bit {@code long} bit pattern.
+     *
+     * <p>
+     * Both non-negative and negative {@code long} values produced by
+     * {@link #writePack7L(OutputStream, long)} are supported. The method consumes
+     * exactly one complete packed value and performs at most ten stream reads.
     *
     * @param in the input stream
     * @return the long value read
-     * @throws IOException if an I/O error occurs or if the stream ends prematurely
+     * @throws EOFException if the stream ends before a terminating byte
+     * @throws IOException  if an I/O error occurs, the encoding exceeds ten bytes,
+     *                      or the payload exceeds 64 bits
     */
    public static long readPack7L(final InputStream in) throws IOException {
-        long result = in.read();
-        if (result > 0x7f) { // NOPMD
-            return result & 0x7fL;
+        long result = 0;
+        int highPayload = 0;
+        for (int bytes = PACKED_HIGH_PAYLOAD_POSITION; bytes <= MAX_PACKED_LONG_BYTES; bytes++) {
+            int current = in.read();
+            if (current < 0) {
+                throw new EOFException("read packed long EOF");
+            }
+            int payload = current & PACKED_PAYLOAD_MASK;
+            if (bytes == PACKED_HIGH_PAYLOAD_POSITION) {
+                highPayload = payload;
+            }
+            boolean terminated = (current & PACKED_TERMINATION_BIT) != 0;
+            if (bytes == MAX_PACKED_LONG_BYTES) {
+                if (!terminated) {
+                    throw new IOException("packed long exceeds ten bytes");
+                }
+                if (highPayload > MAX_PACKED_LONG_HIGH_PAYLOAD) {
+                    throw new IOException("packed long exceeds 64 bits");
+                }
+            }
+            result = (result << 7) | payload;
+            if (terminated) {
+                return result;
+            }
        }
-        int i;
-        for (i = in.read(); i < 0x80; i = in.read()) {
-            result = (result << 7) | i;
-        }
-        return (result << 7) | (i & 0x7f);
+        throw new IOException("packed long exceeds ten bytes");
    }

    /**
--- a/lib/src/test/java/zeroecho/core/io/UtilTest.java
+++ b/lib/src/test/java/zeroecho/core/io/UtilTest.java
@@ -42,7 +42,10 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.EOFException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Random;
 import java.util.UUID;

@@ -143,6 +146,83 @@ public class UtilTest {
        System.out.println("...ok");
    }

+    @Test
+    public void packedLongBoundaryRoundTripsAndConsumesExactly() throws IOException {
+        System.out.println("packedLongBoundaryRoundTripsAndConsumesExactly");
+        List<Long> values = packedLongBoundaryValues();
+        for (long value : values) {
+            byte[] encoded = encodePackedLong(value);
+            assertEquals(canonicalPackedLongWidth(value), encoded.length, "Writer must use the canonical width");
+
+            byte[] framed = new byte[encoded.length + 2];
+            System.arraycopy(encoded, 0, framed, 0, encoded.length);
+            framed[encoded.length] = 0x55;
+            framed[encoded.length + 1] = (byte) 0xaa;
+            ByteArrayInputStream input = new ByteArrayInputStream(framed);
+            assertEquals(value, Util.readPack7L(input), "Packed long boundary value should round trip");
+            assertEquals(0x55, input.read(), "Decoder must leave the first sentinel unread");
+            assertEquals(0xaa, input.read(), "Decoder must leave the second sentinel unread");
+        }
+
+        ByteArrayInputStream nonCanonicalZero = new ByteArrayInputStream(new byte[] { 0x00, (byte) 0x80, 0x33 });
+        assertEquals(0L, Util.readPack7L(nonCanonicalZero), "Existing non-minimal encodings must remain readable");
+        assertEquals(0x33, nonCanonicalZero.read(), "Non-minimal decoding must consume exactly one value");
+        System.out.println("...boundary values=" + values.size());
+        System.out.println("...ok");
+    }
+
+    @Test
+    public void packedLongRejectsInitialAndContinuationEof() throws IOException {
+        System.out.println("packedLongRejectsInitialAndContinuationEof");
+        EOFException initial = assertThrows(EOFException.class,
+                () -> Util.readPack7L(new GuardedInputStream(new byte[0], 1)));
+        assertEquals("read packed long EOF", initial.getMessage());
+
+        byte[] maximumWidth = encodePackedLong(-1L);
+        for (int length = 1; length < maximumWidth.length; length++) {
+            byte[] truncated = new byte[length];
+            System.arraycopy(maximumWidth, 0, truncated, 0, length);
+            EOFException failure = assertThrows(EOFException.class,
+                    () -> Util.readPack7L(new GuardedInputStream(truncated, truncated.length + 1)));
+            assertEquals("read packed long EOF", failure.getMessage());
+        }
+
+        byte[] maximumUnterminated = new byte[10];
+        IOException width = assertThrows(IOException.class,
+                () -> Util.readPack7L(new GuardedInputStream(maximumUnterminated, maximumUnterminated.length)));
+        assertEquals("packed long exceeds ten bytes", width.getMessage());
+        System.out.println("...truncated prefixes=10");
+        System.out.println("...ok");
+    }
+
+    @Test
+    public void packedLongRejectsWidthAndOverflowWithinTenReads() throws IOException {
+        System.out.println("packedLongRejectsWidthAndOverflowWithinTenReads");
+        byte[] overflow = new byte[10];
+        overflow[0] = 0x02;
+        overflow[overflow.length - 1] = (byte) 0xff;
+        GuardedInputStream overflowInput = new GuardedInputStream(overflow, 10);
+        IOException overflowFailure = assertThrows(IOException.class, () -> Util.readPack7L(overflowInput));
+        assertEquals("packed long exceeds 64 bits", overflowFailure.getMessage());
+        assertEquals(10, overflowInput.readCount(), "Overflow rejection must use the fixed read bound");
+
+        byte[] overWidth = new byte[11];
+        overWidth[overWidth.length - 1] = (byte) 0x80;
+        GuardedInputStream overWidthInput = new GuardedInputStream(overWidth, 10);
+        IOException widthFailure = assertThrows(IOException.class, () -> Util.readPack7L(overWidthInput));
+        assertEquals("packed long exceeds ten bytes", widthFailure.getMessage());
+        assertEquals(10, overWidthInput.readCount(), "Decoder must not consume an eleventh byte");
+        assertEquals(10, overWidthInput.position(), "The following byte must remain unread");
+
+        byte[] oldWraparound = new byte[] { 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f, 0x7f,
+                (byte) 0xff };
+        IOException wrapFailure = assertThrows(IOException.class,
+                () -> Util.readPack7L(new GuardedInputStream(oldWraparound, 10)));
+        assertEquals("packed long exceeds 64 bits", wrapFailure.getMessage());
+        System.out.println("...maximum decoder reads=10");
+        System.out.println("...ok");
+    }
+
    @Test
    public void testReadEOFHandling() throws IOException {
        System.out.println("testReadEOFHandling");
@@ -163,4 +243,68 @@ public class UtilTest {
        assertArrayEquals(data, result, "Large byte array should be preserved");
        System.out.println("...ok");
    }
+
+    private static List<Long> packedLongBoundaryValues() {
+        List<Long> values = new ArrayList<>();
+        values.add(Long.MIN_VALUE);
+        values.add(Long.MIN_VALUE + 1);
+        values.add(-1L);
+        values.add(0L);
+        values.add(1L);
+        for (int bits = 7; bits <= 56; bits += 7) {
+            long transition = 1L << bits;
+            values.add(transition - 1);
+            values.add(transition);
+            values.add(transition + 1);
+        }
+        values.add(Long.MAX_VALUE);
+        return values;
+    }
+
+    private static byte[] encodePackedLong(long value) throws IOException {
+        ByteArrayOutputStream output = new ByteArrayOutputStream();
+        Util.writePack7L(output, value);
+        return output.toByteArray();
+    }
+
+    private static int canonicalPackedLongWidth(long value) {
+        int width = 1;
+        while ((value & ~0x7fL) != 0) {
+            width++;
+            value >>>= 7;
+        }
+        return width;
+    }
+
+    private static final class GuardedInputStream extends InputStream {
+        private final byte[] data;
+        private final int maximumReads;
+        private int position;
+        private int readCount;
+
+        private GuardedInputStream(byte[] data, int maximumReads) {
+            this.data = data.clone();
+            this.maximumReads = maximumReads;
+        }
+
+        @Override
+        public int read() {
+            if (readCount >= maximumReads) {
+                throw new AssertionError("decoder exceeded the permitted read bound");
+            }
+            readCount++;
+            if (position >= data.length) {
+                return -1;
+            }
+            return data[position++] & 0xff;
+        }
+
+        private int position() {
+            return position;
+        }
+
+        private int readCount() {
+            return readCount;
+        }
+    }
 }