From 1416425a32e086e20b7d726e9c17332f33b138c3 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Fri, 28 Aug 2026 09:32:02 +0100 Subject: [PATCH 1/4] Check the declared payload length against the bytes present before allocating Io.Reader.readByteArray took the sender's declared payload count and passed it to readBytes, which checked the count against the end of the buffer before sizing the copy, so this port never allocated from the declared number alone. What it did not check was that the payload is followed by exactly the 64 byte signature and nothing else. A declared count short of the bytes present parsed, with the signature read from the middle of the envelope and the remainder ignored, and an envelope with bytes after the signature parsed with those bytes ignored. The reference fix in owid-dotnet sets the rule every port now follows, so the same malformed envelopes are refused the same way everywhere. The count is now checked against the bytes present before anything is sized by it. A valid OWID is the declared payload followed by the 64 byte signature and nothing else, so the count must equal the bytes remaining less the signature length, and any other count, short or long, is refused with the existing OwidException, which names the declared length and the bytes present. Envelopes with a byte after the signature, previously ignored, are now refused as malformed. The separate Integer.MAX_VALUE check is gone because the new check covers it. The other length driven reads were checked and none needed changing, being the domain terminator scan, which stops at the end of the buffer, and the date and signature reads, which go through the bounded readBytes. PayloadLengthTest covers a matching envelope, the library's own signed output, off-by-one counts, a trailing byte, a short signature, declared lengths of 64 MiB, 2 GiB and 0xFFFFFFFF each refused with under 64 KiB allocated on the thread, and an empty payload. The Javadoc on Owid.fromByteArray and on the serialized form now say the signature ends the envelope. Files changed: src/main/java/com/swancommunity/owid/Io.java src/main/java/com/swancommunity/owid/Owid.java src/test/java/com/swancommunity/owid/PayloadLengthTest.java --- src/main/java/com/swancommunity/owid/Io.java | 25 +- .../java/com/swancommunity/owid/Owid.java | 9 +- .../swancommunity/owid/PayloadLengthTest.java | 218 ++++++++++++++++++ 3 files changed, 244 insertions(+), 8 deletions(-) create mode 100644 src/test/java/com/swancommunity/owid/PayloadLengthTest.java diff --git a/src/main/java/com/swancommunity/owid/Io.java b/src/main/java/com/swancommunity/owid/Io.java index 17de59d..38fbdda 100644 --- a/src/main/java/com/swancommunity/owid/Io.java +++ b/src/main/java/com/swancommunity/owid/Io.java @@ -73,6 +73,11 @@ int readByte() throws OwidException { return buffer[position++] & 0xFF; } + /** + * Copies the next count bytes. The end of the buffer is checked + * before the copy is sized, so a count beyond the bytes present is + * refused without allocating. + */ private byte[] readBytes(int count) throws OwidException { if (count < 0) { throw new OwidException("payload length is negative"); @@ -117,14 +122,24 @@ long readUInt32() throws OwidException { } /** - * Reads a byte array prefixed with its length as an unsigned 32 bit - * integer. + * Reads the length prefixed payload. The count is whatever the sender + * declared, so it is checked against the bytes actually present + * before anything is sized by it. A valid OWID is the declared + * payload followed by the signature and nothing else, so the count + * must equal the bytes remaining less the signature length, and any + * other count, short or long, is refused here. The same check refuses + * an envelope with bytes after the signature, which was previously + * accepted and ignored, and one whose signature is short. */ byte[] readByteArray() throws OwidException { long count = readUInt32(); - if (count > Integer.MAX_VALUE) { - throw new OwidException("payload length '" + count - + "' exceeds the maximum supported length"); + long remaining = (long) buffer.length - position; + long expected = count + Owid.SIGNATURE_LENGTH; + if (remaining != expected) { + throw new OwidException("OWID payload length '" + count + + "' does not match the '" + remaining + + "' bytes present, of which the final '" + + Owid.SIGNATURE_LENGTH + "' must be the signature"); } return readBytes((int) count); } diff --git a/src/main/java/com/swancommunity/owid/Owid.java b/src/main/java/com/swancommunity/owid/Owid.java index 6d69221..3db8ea1 100644 --- a/src/main/java/com/swancommunity/owid/Owid.java +++ b/src/main/java/com/swancommunity/owid/Owid.java @@ -42,7 +42,8 @@ *
  • date: four little endian bytes counting minutes since * 2020-01-01 UTC (two big endian bytes counting hours for version 1).
    • *
  • payload: a four byte little endian length followed by the bytes.
    • - *
  • signature: 64 bytes, the r and s values concatenated.
    • + *
  • signature: 64 bytes, the r and s values concatenated. Nothing + * follows the signature.
    • * */ public final class Owid { @@ -106,8 +107,10 @@ public static Owid fromBase64(String value) throws OwidException { * * @param buffer the serialized OWID bytes * @return the parsed OWID - * @throws OwidException if the first byte is not a known version, or the - * buffer is too short for the remaining fields + * @throws OwidException if the first byte is not a known version, the + * buffer is too short for the remaining fields, or + * the declared payload length does not leave + * exactly the 64 byte signature at the end */ public static Owid fromByteArray(byte[] buffer) throws OwidException { return fromReader(new Io.Reader(buffer)); diff --git a/src/test/java/com/swancommunity/owid/PayloadLengthTest.java b/src/test/java/com/swancommunity/owid/PayloadLengthTest.java new file mode 100644 index 0000000..b8611c1 --- /dev/null +++ b/src/test/java/com/swancommunity/owid/PayloadLengthTest.java @@ -0,0 +1,218 @@ +/* **************************************************************************** + * Copyright 2026 51 Degrees Mobile Experts Limited (51degrees.com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * ***************************************************************************/ + +package com.swancommunity.owid; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadMXBean; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +/** + * The payload length field of an OWID is whatever the sender declared, so + * parsing must check it against the bytes present before sizing anything by + * it. These tests prove that a declared length that does not leave exactly + * the signature after the payload is refused, that refusing it costs no + * allocation sized by the declared number, and that a correctly sized + * envelope still parses. The 64 byte signature is the fixed tail every valid + * OWID ends with. + */ +class PayloadLengthTest { + + private static final int SIGNATURE_LENGTH = Owid.SIGNATURE_LENGTH; + + private static final String DOMAIN = "51d.es"; + + private static final byte[] PAYLOAD = filled(37, (byte) 0x5A); + + private static final byte[] SIGNATURE = + filled(SIGNATURE_LENGTH, (byte) 0x99); + + /** The refusal of any declared length must allocate under this. */ + private static final long ALLOCATION_BOUND = 64L * 1024; + + /** + * A version 3 envelope, being the version byte, the domain with its + * terminator, four minute bytes, the declared payload length, the + * payload bytes given and the signature bytes given, so a test can make + * the declared length and the bytes present disagree. The bytes are + * written by hand rather than through the library so the test does not + * depend on the writer it is checking the reader against. + */ + private static byte[] envelope( + long declaredLength, byte[] payload, byte[] signature) { + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + stream.write(Version.VERSION3.asByte()); + byte[] domain = DOMAIN.getBytes(StandardCharsets.US_ASCII); + stream.write(domain, 0, domain.length); + stream.write(0); + writeLittleEndian(stream, 1000L); + writeLittleEndian(stream, declaredLength); + stream.write(payload, 0, payload.length); + stream.write(signature, 0, signature.length); + return stream.toByteArray(); + } + + private static void writeLittleEndian( + ByteArrayOutputStream stream, long value) { + stream.write((int) (value & 0xFF)); + stream.write((int) ((value >> 8) & 0xFF)); + stream.write((int) ((value >> 16) & 0xFF)); + stream.write((int) ((value >> 24) & 0xFF)); + } + + private static byte[] filled(int length, byte value) { + byte[] bytes = new byte[length]; + Arrays.fill(bytes, value); + return bytes; + } + + /** + * Bytes allocated so far on the current thread, from the HotSpot thread + * bean. The test fails rather than passing silently if the runtime + * cannot measure allocation, because the allocation bound is the point + * of the test that calls this. + */ + private static long allocatedBytes() { + ThreadMXBean bean = ManagementFactory.getThreadMXBean(); + assertTrue(bean instanceof com.sun.management.ThreadMXBean, + "the runtime must be able to measure thread allocation"); + com.sun.management.ThreadMXBean sun = + (com.sun.management.ThreadMXBean) bean; + assertTrue(sun.isThreadAllocatedMemoryEnabled(), + "thread allocation measurement must be enabled"); + return sun.getThreadAllocatedBytes(Thread.currentThread().getId()); + } + + /** + * The declared length matches the bytes present, the signature is the + * last 64 bytes, and the envelope parses to the same payload. + */ + @Test + void declaredLengthMatchesParses() throws OwidException { + Owid owid = Owid.fromByteArray( + envelope(PAYLOAD.length, PAYLOAD, SIGNATURE)); + assertArrayEquals(PAYLOAD, owid.getPayload(), + "should read the payload back unchanged"); + assertArrayEquals(SIGNATURE, owid.getSignature(), + "should read the signature back unchanged"); + assertEquals(DOMAIN, owid.getDomain(), "should read the domain"); + } + + /** + * An OWID signed through the library's own creator still parses from + * its own serialised bytes, so the check agrees with what the library + * itself produces. + */ + @Test + void libraryOutputParses() throws OwidException { + Crypto crypto = Crypto.generate(); + Creator creator = Creator.create(DOMAIN, crypto); + Owid original = creator.signBytes(PAYLOAD); + Owid parsed = Owid.fromByteArray(original.asByteArray()); + assertArrayEquals(PAYLOAD, parsed.getPayload(), + "should read the payload the library wrote"); + assertEquals(original, parsed, "should parse to an equal OWID"); + assertTrue(parsed.verifyWithCrypto(crypto, Collections.emptyList()), + "the parsed OWID should still verify"); + } + + /** + * One more or one fewer than the bytes present is refused, because + * either leaves something other than exactly the signature at the end. + */ + @Test + void declaredLengthOffByOneRefused() { + int[] declaredLengths = {PAYLOAD.length - 1, PAYLOAD.length + 1}; + for (int declared : declaredLengths) { + byte[] bytes = envelope(declared, PAYLOAD, SIGNATURE); + assertThrows(OwidException.class, () -> Owid.fromByteArray(bytes), + "should refuse a declared length of " + declared); + } + } + + /** + * A byte after the signature is refused, because the signature must be + * the end of the envelope. Before the check this byte was ignored. + */ + @Test + void trailingByteAfterSignatureRefused() { + byte[] bytes = envelope(PAYLOAD.length, PAYLOAD, SIGNATURE); + byte[] longer = Arrays.copyOf(bytes, bytes.length + 1); + assertThrows(OwidException.class, () -> Owid.fromByteArray(longer), + "should refuse a byte after the signature"); + } + + /** + * A short signature is refused. The declared payload length is right + * for the payload, but the bytes after it are fewer than a signature. + */ + @Test + void shortSignatureRefused() { + byte[] bytes = envelope(PAYLOAD.length, PAYLOAD, + filled(SIGNATURE_LENGTH - 1, (byte) 0x99)); + assertThrows(OwidException.class, () -> Owid.fromByteArray(bytes), + "should refuse a 63 byte signature"); + } + + /** + * A declared length far beyond the bytes present is refused without an + * allocation sized by the declared number. The envelope is a few dozen + * bytes and declares 64 MiB, then 2 GiB, then the largest unsigned 32 + * bit value, and each parse allocates under 64 KiB. Measuring the bytes + * allocated on this thread is what proves the refusal happened before + * any array was sized from the declared number. + */ + @Test + void hugeDeclaredLengthRefusedWithoutAllocating() { + long[] declaredLengths = { + 64L * 1024 * 1024, + 0x7FFFFFFFL, + 0xFFFFFFFFL, + }; + for (long declared : declaredLengths) { + byte[] bytes = envelope(declared, new byte[0], new byte[0]); + long before = allocatedBytes(); + assertThrows(OwidException.class, () -> Owid.fromByteArray(bytes), + "should refuse a declared length of " + declared); + long allocated = allocatedBytes() - before; + assertTrue(allocated < ALLOCATION_BOUND, "declared " + declared + + " allocated " + allocated + " bytes"); + } + } + + /** + * A declared length of zero with nothing but the signature after it + * parses to an empty payload, so the check does not refuse the smallest + * valid envelope. + */ + @Test + void emptyPayloadParses() throws OwidException { + Owid owid = Owid.fromByteArray(envelope(0, new byte[0], SIGNATURE)); + assertEquals(0, owid.getPayload().length, + "should read an empty payload"); + assertArrayEquals(SIGNATURE, owid.getSignature(), + "should read the signature after the empty payload"); + } +} From 3bbadcbbdd7e8f0913b3f57bd7ad8e946f0604e1 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Fri, 28 Aug 2026 17:48:02 +0100 Subject: [PATCH 2/4] Clarify and optimize large payload handling --- README.md | 28 ++++++ src/main/java/com/swancommunity/owid/Io.java | 9 +- .../java/com/swancommunity/owid/Owid.java | 89 ++++++++++++++++++- .../swancommunity/owid/PayloadLengthTest.java | 31 +++++-- 4 files changed, 141 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 15a71f3..8efaba1 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,34 @@ creates, signs, serializes, and verifies OWIDs. - The well known end point helpers return paths and bodies. They do not bind to any web framework. +## Payload size and application limits + +The OWID wire format stores the payload length as an unsigned 32 bit value, +so a payload from zero through 4,294,967,295 bytes is structurally valid. The +format defines no smaller payload limit. The null-terminated domain has no +separate encoded maximum, so the protocol alone is not an application input +limit for the complete envelope. + +This library validates that the declared payload length agrees with the bytes +present before it sizes or copies the payload. A large declaration without +the corresponding bytes is malformed and is rejected without allocating the +declared size. A matching large payload is not malformed merely because it is +large, and parsing work and memory use scale with the bytes actually present. + +The in-memory APIs remain subject to Java's signed `int` array indexing, +address-space and available-memory limits, so a single Java byte array cannot +represent every value allowed by the unsigned wire field. Applications +accepting untrusted OWIDs must choose limits suitable for their use case and +enforce them before buffering the binary form or decoding Base64. An +implementation capacity failure or an application policy rejection is +distinct from an invalid OWID. + +For transport input, limit the complete HTTP body or encoded envelope; allow +for the domain and other OWID fields as well as the payload. After parsing, +`owid.getPayloadLength()` reports the actual payload size without copying it +and can be used for downstream policy. The parser cannot choose either limit +on behalf of the application. + ## Installation Build and install with Maven. The project targets Java 21. diff --git a/src/main/java/com/swancommunity/owid/Io.java b/src/main/java/com/swancommunity/owid/Io.java index 38fbdda..a98e740 100644 --- a/src/main/java/com/swancommunity/owid/Io.java +++ b/src/main/java/com/swancommunity/owid/Io.java @@ -114,11 +114,10 @@ String readString() throws OwidException { /** Reads an unsigned 32 bit integer in little endian byte order. */ long readUInt32() throws OwidException { - byte[] bytes = readBytes(4); - return ((long) (bytes[0] & 0xFF)) - | ((long) (bytes[1] & 0xFF) << 8) - | ((long) (bytes[2] & 0xFF) << 16) - | ((long) (bytes[3] & 0xFF) << 24); + return ((long) readByte()) + | ((long) readByte() << 8) + | ((long) readByte() << 16) + | ((long) readByte() << 24); } /** diff --git a/src/main/java/com/swancommunity/owid/Owid.java b/src/main/java/com/swancommunity/owid/Owid.java index 3db8ea1..8d2a592 100644 --- a/src/main/java/com/swancommunity/owid/Owid.java +++ b/src/main/java/com/swancommunity/owid/Owid.java @@ -142,9 +142,10 @@ static Owid fromReader(Io.Reader reader) throws OwidException { * be encoded */ public byte[] asByteArray() throws OwidException { - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + ExactByteArrayOutputStream buffer = + new ExactByteArrayOutputStream(byteCount(true)); toBuffer(buffer); - return buffer.toByteArray(); + return buffer.toExactByteArray(); } /** @@ -190,12 +191,82 @@ void toBufferNoSignature(ByteArrayOutputStream buffer) throws OwidException { * form of each of the others in the order provided. */ byte[] dataForCrypto(List others) throws OwidException { - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + int length = byteCount(false); + for (Owid other : others) { + length = addLength(length, other.byteCount(true)); + } + ExactByteArrayOutputStream buffer = + new ExactByteArrayOutputStream(length); toBufferNoSignature(buffer); for (Owid other : others) { other.toBuffer(buffer); } - return buffer.toByteArray(); + return buffer.toExactByteArray(); + } + + /** + * The exact number of bytes serialization will write. + */ + private int byteCount(boolean includeSignature) throws OwidException { + int dateLength; + switch (version) { + case VERSION1: + dateLength = 2; + break; + case VERSION2: + case VERSION3: + dateLength = 4; + break; + default: + throw new OwidException( + "OWID version '" + version + "' not supported"); + } + int length = 1; + length = addLength(length, + domain.getBytes(StandardCharsets.UTF_8).length); + length = addLength(length, 1); + length = addLength(length, dateLength); + length = addLength(length, 4); + length = addLength(length, payload.length); + if (includeSignature) { + if (signature.length != SIGNATURE_LENGTH) { + throw Io.invalidSignatureLength(signature.length); + } + length = addLength(length, SIGNATURE_LENGTH); + } + return length; + } + + /** + * Adds serialized lengths without allowing signed int overflow to turn + * an implementation capacity failure into a malformed OWID. + */ + private static int addLength(int left, int right) throws OwidException { + if (right < 0 || left > Integer.MAX_VALUE - right) { + throw new OwidException( + "OWID byte length exceeds Java array capacity"); + } + return left + right; + } + + /** + * A byte stream whose backing array is already the exact final size. + * Returning that array avoids ByteArrayOutputStream's final full copy. + */ + private static final class ExactByteArrayOutputStream + extends ByteArrayOutputStream { + + ExactByteArrayOutputStream(int size) { + super(size); + } + + byte[] toExactByteArray() throws OwidException { + if (count != buf.length) { + throw new OwidException( + "serialized OWID length did not match its fields"); + } + return buf; + } } /** @@ -340,6 +411,16 @@ public byte[] getPayload() { return payload.clone(); } + /** + * Returns the payload length without copying the payload. This is useful + * when applying a use-case-specific size policy after parsing. + * + * @return the payload length in bytes + */ + public int getPayloadLength() { + return payload.length; + } + /** * Sets the payload bytes. * diff --git a/src/test/java/com/swancommunity/owid/PayloadLengthTest.java b/src/test/java/com/swancommunity/owid/PayloadLengthTest.java index b8611c1..7630d8d 100644 --- a/src/test/java/com/swancommunity/owid/PayloadLengthTest.java +++ b/src/test/java/com/swancommunity/owid/PayloadLengthTest.java @@ -120,6 +120,22 @@ void declaredLengthMatchesParses() throws OwidException { assertEquals(DOMAIN, owid.getDomain(), "should read the domain"); } + /** + * A payload materially larger than an ordinary identifier remains valid + * when its declaration and bytes agree. Size policy belongs to the + * application rather than format parsing. + */ + @Test + void matchingOneMebibytePayloadParses() throws OwidException { + byte[] payload = filled(1024 * 1024, (byte) 0x5A); + + Owid owid = Owid.fromByteArray( + envelope(payload.length, payload, SIGNATURE)); + + assertEquals(payload.length, owid.getPayloadLength()); + assertArrayEquals(payload, owid.getPayload()); + } + /** * An OWID signed through the library's own creator still parses from * its own serialised bytes, so the check agrees with what the library @@ -177,15 +193,16 @@ void shortSignatureRefused() { } /** - * A declared length far beyond the bytes present is refused without an - * allocation sized by the declared number. The envelope is a few dozen - * bytes and declares 64 MiB, then 2 GiB, then the largest unsigned 32 - * bit value, and each parse allocates under 64 KiB. Measuring the bytes - * allocated on this thread is what proves the refusal happened before - * any array was sized from the declared number. + * A large declaration whose payload bytes are absent is refused without + * an allocation sized by the declared number. The envelope is a few + * dozen bytes while declaring 64 MiB, then 2 GiB, then the largest + * unsigned 32 bit value, and each parse allocates under 64 KiB. The + * numeric values remain valid when the matching payload is present. + * Measuring the bytes allocated on this thread proves the refusal + * happened before any array was sized from the declaration. */ @Test - void hugeDeclaredLengthRefusedWithoutAllocating() { + void mismatchedLargeDeclarationRefusedWithoutAllocating() { long[] declaredLengths = { 64L * 1024 * 1024, 0x7FFFFFFFL, From b7197945afdc425fb3671b44d77722128daac143 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sun, 30 Aug 2026 13:22:37 +0100 Subject: [PATCH 3/4] Bound the domain read at the published maximum The creator domain is stored as text followed by a zero terminator, and the reader found the end of it by walking forward to that terminator. A missing or corrupted terminator sent the walk to the end of the buffer, so the cost of parsing was set by the length of the input rather than by the size of the field, which is the same class of attacker controlled work as the declared payload length this branch already checks. RFC 1035 section 2.3.4, "Size limits", restricts the total length of a domain name to 255 octets or less. That number counts the wire format, which spends one length octet on every label and one zero octet on the root. An OWID stores the presentation form instead, being the text "example.com", where the dots stand in for the label length octets and the root has no text at all, so the same published limit is two characters shorter here. The new constant MAXIMUM_DOMAIN_LENGTH in Io carries that reasoning next to the number. The reader now stops at the maximum rather than at the end of the buffer, so an unterminated field costs no more than the maximum however long the buffer is, and a domain over the maximum is refused without reading past it. The refusal uses the existing OwidException with a new message, because the module has one exception type and no error variants to add to. Nothing about a valid envelope changes. New tests cover a domain at the maximum parsing and round tripping, one character over being refused, a buffer with no terminator at all being refused, a sixteen mebibyte domain field being refused while allocating under 64 KiB, and the library's own signed output still parsing and verifying. The suite goes from 43 to 48 tests, all passing. --- src/main/java/com/swancommunity/owid/Io.java | 27 ++- .../java/com/swancommunity/owid/Owid.java | 11 +- .../swancommunity/owid/DomainLengthTest.java | 215 ++++++++++++++++++ 3 files changed, 247 insertions(+), 6 deletions(-) create mode 100644 src/test/java/com/swancommunity/owid/DomainLengthTest.java diff --git a/src/main/java/com/swancommunity/owid/Io.java b/src/main/java/com/swancommunity/owid/Io.java index a98e740..be618b4 100644 --- a/src/main/java/com/swancommunity/owid/Io.java +++ b/src/main/java/com/swancommunity/owid/Io.java @@ -39,6 +39,18 @@ final class Io { */ static final long BASE_DATE_EPOCH_SECONDS = 1_577_836_800L; + /** + * The longest domain an OWID can hold, in characters. RFC 1035 section + * 2.3.4, "Size limits", restricts the total length of a domain name to + * 255 octets or less, and that limit counts the wire format, which + * spends one length octet on every label and one zero octet on the + * root. An OWID stores the presentation form instead, being the text + * "example.com", where the dots stand in for the label length octets + * and the root has no text at all, so the same published limit is two + * characters shorter here. + */ + static final int MAXIMUM_DOMAIN_LENGTH = 253; + private Io() { } @@ -93,17 +105,28 @@ private byte[] readBytes(int count) throws OwidException { } /** - * Reads bytes until the null terminator and returns them as a string. + * Reads the domain, being the bytes up to the null terminator. The + * search stops at {@link Io#MAXIMUM_DOMAIN_LENGTH} rather than at + * the end of the buffer, so a buffer whose terminator is missing or + * corrupted costs no more than the published maximum however long + * that buffer is, and a domain longer than the maximum is refused + * without reading past it. */ String readString() throws OwidException { + long lastTerminator = (long) position + MAXIMUM_DOMAIN_LENGTH; + int limit = (int) Math.min(buffer.length - 1L, lastTerminator); int terminator = -1; - for (int i = position; i < buffer.length; i++) { + for (int i = position; i <= limit; i++) { if (buffer[i] == 0) { terminator = i; break; } } if (terminator < 0) { + if (lastTerminator < buffer.length) { + throw new OwidException("domain is longer than the '" + + MAXIMUM_DOMAIN_LENGTH + "' character maximum"); + } throw endOfBuffer(); } String value = new String(buffer, position, terminator - position, diff --git a/src/main/java/com/swancommunity/owid/Owid.java b/src/main/java/com/swancommunity/owid/Owid.java index 8d2a592..b9cd6ba 100644 --- a/src/main/java/com/swancommunity/owid/Owid.java +++ b/src/main/java/com/swancommunity/owid/Owid.java @@ -38,7 +38,8 @@ * *
      *
    • version: a single byte.
      • - *
    • domain: the UTF-8 bytes of the domain, null terminated.
      • + *
    • domain: the UTF-8 bytes of the domain, null terminated, no longer + * than the maximum published for a domain name.
      • *
    • date: four little endian bytes counting minutes since * 2020-01-01 UTC (two big endian bytes counting hours for version 1).
      • *
    • payload: a four byte little endian length followed by the bytes.
      • @@ -108,9 +109,11 @@ public static Owid fromBase64(String value) throws OwidException { * @param buffer the serialized OWID bytes * @return the parsed OWID * @throws OwidException if the first byte is not a known version, the - * buffer is too short for the remaining fields, or - * the declared payload length does not leave - * exactly the 64 byte signature at the end + * buffer is too short for the remaining fields, the + * domain is unterminated or longer than the maximum + * published for a domain name, or the declared + * payload length does not leave exactly the 64 byte + * signature at the end */ public static Owid fromByteArray(byte[] buffer) throws OwidException { return fromReader(new Io.Reader(buffer)); diff --git a/src/test/java/com/swancommunity/owid/DomainLengthTest.java b/src/test/java/com/swancommunity/owid/DomainLengthTest.java new file mode 100644 index 0000000..ae8f8b1 --- /dev/null +++ b/src/test/java/com/swancommunity/owid/DomainLengthTest.java @@ -0,0 +1,215 @@ +/* **************************************************************************** + * Copyright 2026 51 Degrees Mobile Experts Limited (51degrees.com) + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * ***************************************************************************/ + +package com.swancommunity.owid; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadMXBean; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +/** + * An OWID stores the creator domain as text followed by a zero terminator, + * and parsing finds the end of the domain by walking forward to that + * terminator. A missing or corrupted terminator once made the walk run to the + * end of the buffer, which is work whose size an attacker chooses. RFC 1035 + * section 2.3.4 publishes a maximum for a domain name, so these tests prove + * that a domain at that maximum still parses, that a longer one is refused, + * that a buffer with no terminator at all is refused, and that refusing a + * hostile buffer costs no more than the maximum however long the buffer is. + */ +class DomainLengthTest { + + private static final int MAXIMUM = Io.MAXIMUM_DOMAIN_LENGTH; + + private static final byte[] PAYLOAD = filled(37, (byte) 0x5A); + + private static final byte[] SIGNATURE = + filled(Owid.SIGNATURE_LENGTH, (byte) 0x99); + + /** The refusal of any domain field must allocate under this. */ + private static final long ALLOCATION_BOUND = 64L * 1024; + + /** + * A domain field long enough that walking it would show plainly in the + * allocation figure, being far more than the published maximum. + */ + private static final int HOSTILE_DOMAIN_LENGTH = 16 * 1024 * 1024; + + /** + * A version 3 envelope carrying the domain bytes given, being the version + * byte, those bytes with a terminator after them, four minute bytes, the + * payload with its declared length, and the signature. The bytes are + * written by hand rather than through the library so the test does not + * depend on the writer it is checking the reader against. + */ + private static byte[] envelope(byte[] domain) { + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + stream.write(Version.VERSION3.asByte()); + stream.write(domain, 0, domain.length); + stream.write(0); + writeLittleEndian(stream, 1000L); + writeLittleEndian(stream, PAYLOAD.length); + stream.write(PAYLOAD, 0, PAYLOAD.length); + stream.write(SIGNATURE, 0, SIGNATURE.length); + return stream.toByteArray(); + } + + private static void writeLittleEndian( + ByteArrayOutputStream stream, long value) { + stream.write((int) (value & 0xFF)); + stream.write((int) ((value >> 8) & 0xFF)); + stream.write((int) ((value >> 16) & 0xFF)); + stream.write((int) ((value >> 24) & 0xFF)); + } + + private static byte[] filled(int length, byte value) { + byte[] bytes = new byte[length]; + Arrays.fill(bytes, value); + return bytes; + } + + /** + * A domain of the length given, written as labels separated by dots so it + * has the shape of a real name rather than one long run of letters. + */ + private static String domainOfLength(int length) { + StringBuilder builder = new StringBuilder(length); + while (builder.length() < length) { + builder.append(builder.length() % 64 == 63 ? '.' : 'a'); + } + return builder.toString(); + } + + private static byte[] ascii(String value) { + return value.getBytes(StandardCharsets.US_ASCII); + } + + /** + * Bytes allocated so far on the current thread, from the HotSpot thread + * bean. The test fails rather than passing silently if the runtime cannot + * measure allocation, because the allocation bound is the point of the + * test that calls this. + */ + private static long allocatedBytes() { + ThreadMXBean bean = ManagementFactory.getThreadMXBean(); + assertTrue(bean instanceof com.sun.management.ThreadMXBean, + "the runtime must be able to measure thread allocation"); + com.sun.management.ThreadMXBean sun = + (com.sun.management.ThreadMXBean) bean; + assertTrue(sun.isThreadAllocatedMemoryEnabled(), + "thread allocation measurement must be enabled"); + return sun.getThreadAllocatedBytes(Thread.currentThread().getId()); + } + + /** + * A domain of exactly the published maximum parses, keeps its value, and + * survives being written back out and read again. + */ + @Test + void maximumLengthDomainParses() throws OwidException { + String domain = domainOfLength(MAXIMUM); + byte[] bytes = envelope(ascii(domain)); + + Owid owid = Owid.fromByteArray(bytes); + + assertEquals(MAXIMUM, owid.getDomain().length(), + "should read a domain of the published maximum length"); + assertEquals(domain, owid.getDomain(), "should read the domain"); + assertArrayEquals(PAYLOAD, owid.getPayload(), + "should read the payload that follows the domain"); + assertArrayEquals(bytes, owid.asByteArray(), + "should write the same bytes back out"); + assertEquals(owid, Owid.fromByteArray(owid.asByteArray()), + "should parse its own output to an equal OWID"); + } + + /** + * One character more than the published maximum is refused, even though + * the terminator and every field after it are present and correct. + */ + @Test + void overMaximumLengthDomainRefused() { + byte[] bytes = envelope(ascii(domainOfLength(MAXIMUM + 1))); + + assertThrows(OwidException.class, () -> Owid.fromByteArray(bytes), + "should refuse a domain one character over the maximum"); + } + + /** + * A buffer whose domain field has no terminator anywhere is refused. The + * buffer holds nothing but the version byte and letters, so there is no + * zero for the walk to stop at. + */ + @Test + void missingTerminatorRefused() { + byte[] bytes = filled(64 * 1024, (byte) 'a'); + bytes[0] = Version.VERSION3.asByte(); + + assertThrows(OwidException.class, () -> Owid.fromByteArray(bytes), + "should refuse a buffer with no terminator"); + } + + /** + * A domain field of sixteen mebibytes, terminated far past the maximum, + * is refused without an allocation sized by the field. Measuring the + * bytes allocated on this thread proves the cost of the refusal is set by + * the published maximum and not by the length of the buffer, because + * building the string the field describes could not fit under the bound. + */ + @Test + void hostileDomainRefusedWithoutAllocating() { + byte[] bytes = envelope(filled(HOSTILE_DOMAIN_LENGTH, (byte) 'a')); + + long before = allocatedBytes(); + assertThrows(OwidException.class, () -> Owid.fromByteArray(bytes), + "should refuse a domain field of " + + HOSTILE_DOMAIN_LENGTH + " bytes"); + long allocated = allocatedBytes() - before; + + assertTrue(allocated < ALLOCATION_BOUND, "refusing a " + + HOSTILE_DOMAIN_LENGTH + " byte domain field allocated " + + allocated + " bytes"); + } + + /** + * An OWID signed through the library's own creator still parses from its + * own serialised bytes and still verifies, so the bound is not + * retrospective on anything the library produces. + */ + @Test + void libraryOutputParses() throws OwidException { + Crypto crypto = Crypto.generate(); + Creator creator = Creator.create("51d.es", crypto); + Owid original = creator.signBytes(PAYLOAD); + + Owid parsed = Owid.fromByteArray(original.asByteArray()); + + assertEquals("51d.es", parsed.getDomain(), + "should read the domain the library wrote"); + assertEquals(original, parsed, "should parse to an equal OWID"); + assertTrue(parsed.verifyWithCrypto(crypto, Collections.emptyList()), + "the parsed OWID should still verify"); + } +} From ae16533acba76d85d61592e81a3969a320544951 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sun, 30 Aug 2026 13:42:21 +0100 Subject: [PATCH 4/4] Refuse to write a domain longer than the maximum Reading an OWID has been bounded at the published maximum for a domain name since the previous commit, but writing one was not, so a creator configured with a longer domain still produced an OWID that this same library would refuse to parse. A library that can emit what it cannot read leaves the fault to surface at the consumer rather than at the creator. The one constant, Io.MAXIMUM_DOMAIN_LENGTH, now bounds both halves. Creator.create refuses a domain over the maximum when the caller supplies it, before the crypto instance is looked at and before any OWID exists, and Io.writeString refuses one that reaches serialisation by another route such as Owid.setDomain. Both raise the OwidException the read raises, through one shared factory, so the two halves report the one condition in the same words. The length counted is the UTF-8 bytes, being what the read counts as it walks to the terminator. Nothing at or under the maximum behaves differently. Four tests cover a creator holding a domain of exactly the maximum signing an OWID that round trips and verifies, a creator refused one character over with the maximum named in the message, the refusal arriving before the private key is used, and serialisation refusing a domain that arrived by another route. Removing only the two new checks fails three of the four. --- .../java/com/swancommunity/owid/Creator.java | 20 +++- src/main/java/com/swancommunity/owid/Io.java | 22 +++- .../swancommunity/owid/DomainLengthTest.java | 104 ++++++++++++++++++ 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/swancommunity/owid/Creator.java b/src/main/java/com/swancommunity/owid/Creator.java index 63b8b16..2d42cc4 100644 --- a/src/main/java/com/swancommunity/owid/Creator.java +++ b/src/main/java/com/swancommunity/owid/Creator.java @@ -44,17 +44,28 @@ private Creator(String domain, Crypto crypto) { * Creates a new creator for the domain using the crypto instance for * signing. * + *

      The domain is refused here if it is longer than the maximum + * published for a domain name, being the same bound parsing applies, so + * a creator can never be built that would produce an OWID this library + * would refuse to read. The refusal arrives when the caller supplies the + * domain rather than later when an OWID is signed.

      + * * @param domain the domain associated with the creator * @param crypto the crypto instance that can sign * @return the creator - * @throws OwidException if the domain is empty or whitespace, or the - * crypto instance cannot sign + * @throws OwidException if the domain is empty or whitespace, is longer + * than the maximum published for a domain name, or + * the crypto instance cannot sign */ public static Creator create(String domain, Crypto crypto) throws OwidException { if (domain == null || domain.trim().isEmpty()) { throw new OwidException("domain '" + domain + "' is not valid"); } + if (domain.getBytes(StandardCharsets.UTF_8).length + > Io.MAXIMUM_DOMAIN_LENGTH) { + throw Io.domainTooLong(); + } if (!crypto.canSign()) { throw new OwidException( "instance of Crypto cannot be used to generate a signature"); @@ -68,8 +79,9 @@ public static Creator create(String domain, Crypto crypto) * @param domain the domain associated with the creator * @param privatePem the private key in PKCS#8 PEM form * @return the creator - * @throws OwidException if the domain is empty, or the PEM is not a valid - * private key + * @throws OwidException if the domain is empty or longer than the + * maximum published for a domain name, or the PEM + * is not a valid private key */ public static Creator fromPrivatePem(String domain, String privatePem) throws OwidException { diff --git a/src/main/java/com/swancommunity/owid/Io.java b/src/main/java/com/swancommunity/owid/Io.java index be618b4..74616ba 100644 --- a/src/main/java/com/swancommunity/owid/Io.java +++ b/src/main/java/com/swancommunity/owid/Io.java @@ -124,8 +124,7 @@ String readString() throws OwidException { } if (terminator < 0) { if (lastTerminator < buffer.length) { - throw new OwidException("domain is longer than the '" - + MAXIMUM_DOMAIN_LENGTH + "' character maximum"); + throw domainTooLong(); } throw endOfBuffer(); } @@ -202,11 +201,18 @@ static void writeByte(ByteArrayOutputStream buffer, byte value) { /** * Writes the string followed by the null terminator. The string must not - * contain a null character as that would conflict with the terminator. + * contain a null character as that would conflict with the terminator, + * and must be no longer than {@link #MAXIMUM_DOMAIN_LENGTH} bytes, being + * the bound the read applies, so the library cannot write a domain it + * would then refuse to read back. The count is of the UTF-8 bytes + * because those are what the read counts as it walks to the terminator. */ static void writeString(ByteArrayOutputStream buffer, String value) throws OwidException { byte[] bytes = value.getBytes(StandardCharsets.UTF_8); + if (bytes.length > MAXIMUM_DOMAIN_LENGTH) { + throw domainTooLong(); + } for (byte b : bytes) { if (b == 0) { throw new OwidException("domain '" + value + "' is not valid"); @@ -275,6 +281,16 @@ static void writeDate(ByteArrayOutputStream buffer, Instant date, } } + /** + * The refusal used by both halves of the library when a domain is longer + * than the published maximum, so the read and the write report the one + * condition in the same words. + */ + static OwidException domainTooLong() { + return new OwidException("domain is longer than the '" + + MAXIMUM_DOMAIN_LENGTH + "' character maximum"); + } + static OwidException invalidSignatureLength(int length) { return new OwidException("signature length '" + length + "' not compatible with '" + Owid.SIGNATURE_LENGTH diff --git a/src/test/java/com/swancommunity/owid/DomainLengthTest.java b/src/test/java/com/swancommunity/owid/DomainLengthTest.java index ae8f8b1..5a8054a 100644 --- a/src/test/java/com/swancommunity/owid/DomainLengthTest.java +++ b/src/test/java/com/swancommunity/owid/DomainLengthTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -38,6 +39,13 @@ * that a domain at that maximum still parses, that a longer one is refused, * that a buffer with no terminator at all is refused, and that refusing a * hostile buffer costs no more than the maximum however long the buffer is. + * + *

      The same maximum binds the write, because a library that can emit an + * OWID it cannot read leaves the fault to surface at the consumer rather + * than at the creator. The later tests prove a creator refuses a domain over + * the maximum when the caller supplies it, before the private key is used at + * all, and that serialising refuses a domain that arrived by any other + * route.

      */ class DomainLengthTest { @@ -193,6 +201,102 @@ void hostileDomainRefusedWithoutAllocating() { + allocated + " bytes"); } + /** + * A creator holding a domain of exactly the published maximum signs an + * OWID that serialises, parses back to the same domain, and verifies, so + * the write bound refuses nothing the library accepted before. + */ + @Test + void maximumLengthDomainWritten() throws OwidException { + String domain = domainOfLength(MAXIMUM); + Crypto crypto = Crypto.generate(); + Creator creator = Creator.create(domain, crypto); + + Owid signed = creator.signBytes(PAYLOAD); + Owid parsed = Owid.fromByteArray(signed.asByteArray()); + + assertEquals(MAXIMUM, parsed.getDomain().length(), + "should write and read a domain of the published maximum"); + assertEquals(domain, parsed.getDomain(), + "should round trip the domain the creator holds"); + assertEquals(signed, parsed, "should parse to an equal OWID"); + assertTrue(parsed.verifyWithCrypto(crypto, Collections.emptyList()), + "the parsed OWID should still verify"); + } + + /** + * A creator is refused one character over the maximum, at the point the + * caller supplies the domain, and the message names the maximum so the + * caller can see what the domain has to fit. + */ + @Test + void overMaximumLengthDomainRefusedByCreator() throws OwidException { + Crypto crypto = Crypto.generate(); + + OwidException thrown = assertThrows(OwidException.class, + () -> Creator.create(domainOfLength(MAXIMUM + 1), crypto), + "should refuse a domain one character over the maximum"); + + assertNamesMaximum(thrown); + } + + /** + * The refusal comes before anything is signed. A creator cannot be built + * with an over long domain even when the crypto instance holds no private + * key at all, and a domain that reaches an OWID by another route is + * refused as the bytes to sign are assembled, which is the step before + * the private key is used, leaving the OWID with no signature. + */ + @Test + void overMaximumLengthDomainRefusedBeforeSigning() throws OwidException { + String domain = domainOfLength(MAXIMUM + 1); + Crypto verifyOnly = + Crypto.newVerifyOnly(Crypto.generate().publicKeyPem()); + assertFalse(verifyOnly.canSign(), + "the crypto instance should not be able to sign"); + + OwidException fromCreator = assertThrows(OwidException.class, + () -> Creator.create(domain, verifyOnly), + "should refuse the domain without reaching the crypto"); + assertNamesMaximum(fromCreator); + + Owid owid = new Owid(); + owid.setDomain(domain); + owid.setPayload(PAYLOAD); + OwidException fromData = assertThrows(OwidException.class, + () -> owid.dataForCrypto(Collections.emptyList()), + "should refuse to assemble the bytes that would be signed"); + assertNamesMaximum(fromData); + assertEquals(0, owid.getSignature().length, + "nothing should have been signed"); + } + + /** + * Serialising refuses a domain over the maximum however it arrived. The + * OWID here carries a signature of the right length, so the refusal is + * the domain and not a missing signature. + */ + @Test + void overMaximumLengthDomainRefusedWhenSerialising() { + Owid owid = new Owid(); + owid.setDomain(domainOfLength(MAXIMUM + 1)); + owid.setPayload(PAYLOAD); + owid.setSignature(SIGNATURE); + + OwidException thrown = assertThrows(OwidException.class, + owid::asByteArray, + "should refuse to serialise a domain over the maximum"); + + assertNamesMaximum(thrown); + } + + /** The refusal has to name the maximum the caller must fit within. */ + private static void assertNamesMaximum(OwidException thrown) { + assertTrue(thrown.getMessage().contains(String.valueOf(MAXIMUM)), + "the message '" + thrown.getMessage() + + "' should name the '" + MAXIMUM + "' maximum"); + } + /** * An OWID signed through the library's own creator still parses from its * own serialised bytes and still verifies, so the bound is not