Harden token-store load and key validation

FileTokenStore.load read the file with Files.readAllBytes after a
separate Files.size check. Because the file is attacker-writable, a
concurrent grow between the two would make readAllBytes allocate
gigabytes and throw OutOfMemoryError - an Error that the best-effort
RuntimeException guard in OidcDeviceAuth.maybeLoadFromStore does not
catch, so a bad file aborted sign-in instead of degrading. A new
readBounded reads through a FileChannel into a buffer capped at the
reported size (already bounded by MAX_FILE_BYTES) plus one byte, so a
file that grew past its reported size is rejected, never allocated.

TokenFileParser tracked only object depth, so an array-wrapped object
([ {..} ]) extracted its fields through the depth gate. The parser now
marks any array as malformed and parseAndVerify rejects a shape that is
not a single flat JSON object, keeping the on-disk format strict.

TokenStoreKey now rejects a null clientId, tokenEndpoint,
deviceAuthorizationEndpoint or scope with a clear OidcAuthException
instead of surfacing a raw NullPointerException deep in a store's
serialize/fingerprint path, and normalises an empty audience to null so
getAudience(), hash() and the save/load round-trip agree that an absent
audience is null (an empty audience previously broke its own round-trip).

parseAndVerify now bulk-copies the file bytes into native memory via a
new DirectUtf8Sink.put(byte[], int, int) instead of a byte-by-byte loop.

Add tests for the array-wrapped reject, the empty-audience
normalisation, and the null-required-field rejection; each was proven to
fail with its fix reverted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

core/src/main/java/io/questdb/client/cutlass/auth/FileTokenStore.java

@@ -228,20 +228,15 @@ public PersistedToken load(TokenStoreKey key) {
         Path file = tokenFile(key);
         byte[] bytes;
         try {
-            if (!Files.exists(file)) {
-                return null;
-            }
-            long size = Files.size(file);
-            if (size <= 0 || size > MAX_FILE_BYTES) {
-                // an empty or implausibly large file is not a usable entry; ignore it rather than read it
-                return null;
-            }
-            bytes = Files.readAllBytes(file);
+            bytes = readBounded(file);
         } catch (NoSuchFileException e) {
             return null;
         } catch (IOException e) {
             throw new OidcAuthException(e).put("could not read the OIDC token store file");
         }
+        if (bytes == null) {
+            return null;
+        }
         return parseAndVerify(key, bytes);
     }
 
@@ -313,10 +308,8 @@ private static PersistedToken parseAndVerify(TokenStoreKey key, byte[] bytes) {
         TokenFileParser parser = new TokenFileParser();
         try (DirectUtf8Sink mem = new DirectUtf8Sink(bytes.length);
              JsonLexer lexer = new JsonLexer(JSON_LEXER_CACHE_SIZE, JSON_LEXER_MAX_VALUE_BYTES)) {
-            for (int i = 0; i < bytes.length; i++) {
-                // putAny accepts any byte; put(byte) is asserted for non-ASCII bytes only
-                mem.putAny(bytes[i]);
-            }
+            // bulk-copy the file bytes into native memory in one go rather than byte by byte
+            mem.put(bytes, 0, bytes.length);
             long lo = mem.ptr();
             lexer.parse(lo, lo + mem.size(), parser);
             lexer.parseLast(); // reject a truncated document
@@ -325,8 +318,9 @@ private static PersistedToken parseAndVerify(TokenStoreKey key, byte[] bytes) {
             return null;
         }
         // schema and fingerprint must match the live identity; a mismatch is a hash collision or a file
-        // copied from a different identity, so ignore it rather than serve the wrong identity's token
-        if (parser.version != SCHEMA_VERSION) {
+        // copied from a different identity, so ignore it rather than serve the wrong identity's token. A
+        // malformed shape (an array anywhere - the schema is a single flat object) is likewise rejected.
+        if (parser.malformed || parser.version != SCHEMA_VERSION) {
             return null;
         }
         if (!Chars.equals(key.getClientId(), parser.clientId)
@@ -411,6 +405,35 @@ private static void putStringMember(StringSink sink, String name, CharSequence v
         putString(sink, value);
     }
 
+    private static byte[] readBounded(Path file) throws IOException {
+        // read with a hard cap instead of Files.readAllBytes after a separate Files.size: the file is
+        // attacker-writable, so a size-check-then-read races a concurrent grow - a file enlarged past the cap
+        // between the two would make readAllBytes allocate gigabytes and throw OutOfMemoryError (an Error, which
+        // the best-effort RuntimeException guard in OidcDeviceAuth.maybeLoadFromStore would not catch, so a bad
+        // file would abort sign-in instead of degrading). Cap the buffer at the reported size (already bounded
+        // by MAX_FILE_BYTES) plus one byte, so a file that grew past its reported size is rejected, not allocated.
+        try (FileChannel channel = FileChannel.open(file, StandardOpenOption.READ)) {
+            long size = channel.size();
+            if (size <= 0 || size > MAX_FILE_BYTES) {
+                // an empty or implausibly large file is not a usable entry; ignore it rather than read it in
+                return null;
+            }
+            ByteBuffer buffer = ByteBuffer.allocate((int) size + 1);
+            while (buffer.hasRemaining() && channel.read(buffer) >= 0) {
+                // read until EOF or the (size + 1)-byte buffer fills
+            }
+            int read = buffer.position();
+            if (read == 0 || read > size) {
+                // empty, or grew past its reported size between the stat and the read: treat as corrupt/hostile
+                return null;
+            }
+            byte[] bytes = new byte[read];
+            buffer.flip();
+            buffer.get(bytes);
+            return bytes;
+        }
+    }
+
     private static void releaseLock(Path lock, String nonce) {
         // release our own lock only: re-read it and delete it solely when it still carries our nonce. A hold
         // that outran lockStaleMillis may have been judged stale and stolen (deleted and recreated) by a peer;
@@ -623,10 +646,17 @@ private static final class TokenFileParser implements JsonParser {
         int version;
         private int depth;
         private int field = FIELD_NONE;
+        private boolean malformed;
 
         @Override
         public void onEvent(int code, CharSequence tag, int position) {
             switch (code) {
+                case JsonLexer.EVT_ARRAY_START:
+                    // the on-disk schema is a single flat JSON object; an array anywhere (for example a
+                    // top-level [ {..} ] wrapper) is a malformed or hostile shape - mark the document invalid
+                    // rather than extract fields from it through the object-depth gate
+                    malformed = true;
+                    break;
                 case JsonLexer.EVT_OBJ_START:
                     depth++;
                     break;

core/src/main/java/io/questdb/client/cutlass/auth/TokenStoreKey.java

@@ -70,13 +70,24 @@ public TokenStoreKey(
             String audience,
             boolean groupsInToken
     ) {
+        // the identity fields are required; reject a null up front with a clear error rather than letting it
+        // surface later as a raw NullPointerException deep inside a TokenStore's serialize/fingerprint path
+        // the identity fields are required; reject a null up front with a clear error rather than letting it
+        // surface later as a raw NullPointerException deep inside a TokenStore's serialize/fingerprint path
+        if (clientId == null || tokenEndpoint == null || deviceAuthorizationEndpoint == null || scope == null) {
+            throw new OidcAuthException(
+                    "clientId, tokenEndpoint, deviceAuthorizationEndpoint and scope are required for a token store key");
+        }
         this.clientId = clientId;
         this.tokenEndpoint = tokenEndpoint;
         this.deviceAuthorizationEndpoint = deviceAuthorizationEndpoint;
         this.scope = scope;
-        this.audience = audience;
+        // normalise an empty audience to null so getAudience(), hash() (which already folds null and "" together
+        // via nullToEmpty), and a TokenStore's save/load round-trip all agree that an absent audience is null -
+        // matching how OidcDeviceAuth builds the key
+        this.audience = audience != null && !audience.isEmpty() ? audience : null;
         this.groupsInToken = groupsInToken;
-        this.hash = computeHash(clientId, tokenEndpoint, deviceAuthorizationEndpoint, scope, audience, groupsInToken);
+        this.hash = computeHash(clientId, tokenEndpoint, deviceAuthorizationEndpoint, scope, this.audience, groupsInToken);
     }
 
     public String getAudience() {

core/src/main/java/io/questdb/client/std/str/DirectUtf8Sink.java

@@ -25,6 +25,7 @@
 package io.questdb.client.std.str;
 
 import io.questdb.client.std.MemoryTag;
+import io.questdb.client.std.Unsafe;
 import io.questdb.client.std.bytes.DirectByteSink;
 import io.questdb.client.std.bytes.NativeByteSink;
 import org.jetbrains.annotations.NotNull;
@@ -103,6 +104,22 @@ public DirectUtf8Sink put(byte b) {
         return this;
     }
 
+    /**
+     * Appends the bytes of {@code src} in {@code [lo, hi)} verbatim in a single bulk copy, rather than byte by
+     * byte. The ascii hint is set to {@code false} conservatively (the bytes are treated as opaque), so callers
+     * that rely on {@link #isAscii()} should not use this overload for ascii-only content.
+     */
+    public DirectUtf8Sink put(byte[] src, int lo, int hi) {
+        final int len = hi - lo;
+        if (len > 0) {
+            setAscii(false);
+            final long dest = sink.ensureCapacity(len);
+            Unsafe.getUnsafe().copyMemory(src, Unsafe.BYTE_OFFSET + lo, null, dest, len);
+            sink.advance(len);
+        }
+        return this;
+    }
+
     @Override
     public DirectUtf8Sink putAny(byte b) {
         setAscii(isAscii() & b >= 0);

core/src/test/java/io/questdb/client/test/cutlass/auth/FileTokenStoreTest.java

@@ -103,6 +103,27 @@ public void testAdvancedConstructorRejectsOverCapAcquireBudget() throws Exceptio
         });
     }
 
+    @Test
+    public void testArrayWrappedJsonReturnsNull() throws Exception {
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            FileTokenStore store = new FileTokenStore(dir);
+            TokenStoreKey key = sampleKey();
+            // a valid entry, then the same object wrapped in a top-level array. The wrapper leaves the
+            // fingerprint fields untouched, so only the non-object-root rejection - not a fingerprint mismatch -
+            // can reject it: the parser must refuse a shape that is not a single flat JSON object
+            store.save(key, sampleToken("ACCESS-1", "REFRESH-1"));
+            Assert.assertNotNull("the plain object must load", store.load(key));
+            byte[] obj = Files.readAllBytes(tokenFile(dir, key));
+            byte[] wrapped = new byte[obj.length + 2];
+            wrapped[0] = '[';
+            System.arraycopy(obj, 0, wrapped, 1, obj.length);
+            wrapped[wrapped.length - 1] = ']';
+            Files.write(tokenFile(dir, key), wrapped);
+            Assert.assertNull("an array-wrapped object must be rejected as a malformed shape", store.load(key));
+        });
+    }
+
     @Test
     public void testAudienceNullVersusEmptyFingerprint() throws Exception {
         assertMemoryLeak(() -> {
@@ -181,6 +202,28 @@ public void testCorruptFileReturnsNull() throws Exception {
         });
     }
 
+    @Test
+    public void testEmptyAudienceNormalizesToNull() throws Exception {
+        assertMemoryLeak(() -> {
+            Path dir = storeDir();
+            FileTokenStore store = new FileTokenStore(dir);
+            // an empty-string audience is normalised to null: getAudience() reports null, it shares the
+            // null-audience identity hash/file, and its save->load round-trips (the pre-fix "" broke its own
+            // round-trip because the writer recorded "audience":"" but the fingerprint treated it as absent)
+            TokenStoreKey emptyAud = new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                    "https://idp.example.com:443/device", "openid", "", false);
+            TokenStoreKey nullAud = new TokenStoreKey("questdb", "https://idp.example.com:443/token",
+                    "https://idp.example.com:443/device", "openid", null, false);
+            Assert.assertNull("an empty audience must normalise to null", emptyAud.getAudience());
+            Assert.assertEquals("null and empty audiences must share one identity hash", nullAud.hash(), emptyAud.hash());
+
+            store.save(emptyAud, sampleToken("ACCESS-1", "REFRESH-1"));
+            PersistedToken loaded = store.load(emptyAud);
+            Assert.assertNotNull("an empty-audience key must load the entry it just saved", loaded);
+            Assert.assertEquals("ACCESS-1", loaded.getAccessToken());
+        });
+    }
+
     @Test
     public void testEmptyFileReturnsNull() throws Exception {
         assertMemoryLeak(() -> {
@@ -666,6 +709,38 @@ public void testSpecialCharactersAndNullsRoundTrip() throws Exception {
         });
     }
 
+    @Test
+    public void testTokenStoreKeyRejectsNullRequiredFields() throws Exception {
+        assertMemoryLeak(() -> {
+            // the identity fields are required; a null must fail fast with a clear OidcAuthException rather than
+            // surface later as a raw NullPointerException inside save()/load() (audience stays optional)
+            try {
+                new TokenStoreKey(null, "https://idp/token", "https://idp/device", "openid", null, false);
+                Assert.fail("a null clientId must be rejected");
+            } catch (OidcAuthException expected) {
+                // required identity field
+            }
+            try {
+                new TokenStoreKey("questdb", null, "https://idp/device", "openid", null, false);
+                Assert.fail("a null tokenEndpoint must be rejected");
+            } catch (OidcAuthException expected) {
+                // required identity field
+            }
+            try {
+                new TokenStoreKey("questdb", "https://idp/token", null, "openid", null, false);
+                Assert.fail("a null deviceAuthorizationEndpoint must be rejected");
+            } catch (OidcAuthException expected) {
+                // required identity field
+            }
+            try {
+                new TokenStoreKey("questdb", "https://idp/token", "https://idp/device", null, null, false);
+                Assert.fail("a null scope must be rejected");
+            } catch (OidcAuthException expected) {
+                // required identity field
+            }
+        });
+    }
+
     @Test
     public void testTruncatedJsonReturnsNull() throws Exception {
         assertMemoryLeak(() -> {