Harden OIDC device-flow input validation

Address the minor hardening findings from the device-flow review.

endpointPathHasEncodedSeparator() out-decoded the endpoint path with a
byte-oriented percentDecodeOnce and scanned each level for %2f/%5c/%25,
but missed encodings it does not resolve - an overlong-UTF-8 %c0%ae or
an IIS-style %u002e that a permissive server decodes to '.'/'/'. Such a
segment, sitting past the issuer prefix, slipped the path scope. A real
OIDC endpoint path is plain ASCII, so reject any '%' or '\' in it
outright; a provider that encodes its path must be configured
explicitly with builder(), which pins the origin only.

adopt() accepted an empty served token (hasOnlyTokenChars("") is
vacuously true) and would serve it as a blank "Bearer " header that
only draws a 401. Reject an empty served token too, so a corrupt or
tampered entry falls through to a refresh or an interactive sign-in.

Endpoint.parse() let Integer.parseInt read a ":+443" port as 443,
slipping the range check, and accepted a backslash in the host that the
WHATWG URL spec folds to '/'. Reject a leading '+' on the port and a
backslash in the host.

Correct the JsonLexer.unescape() default-case comment: a '\' before a
recognized escape letter is still decoded, so the "Windows path is not
dropped" note held only for genuinely unknown escapes.

Add regression tests for each - overlong-UTF-8 / %u endpoint paths, an
empty served token, and the +port / backslash-host cases; every one
fails on the reverted production line and passes with the fix.

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

Author: glasstiger

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

@@ -653,32 +653,22 @@ private static OidcAuthException endpointOriginNotPinned(String label, String ur
     }
 
     private static boolean endpointPathHasEncodedSeparator(String rawEndpointPath) {
-        // Scan for a literal backslash (decodePathSegments folds it to '/') or a percent-encoded path
-        // separator - %2f ('/'), %5c ('\'), or an encoded percent %25 that gates a split or double encoding
-        // such as %2%66 or %252f - at every decode level, not just the raw string. A separator that only
-        // emerges after the server unescapes more than once would pass a single-pass scan yet split one
-        // segment in two, letting .../realms/acme%2%66evil/token slip the issuer-path scope. A real OIDC
-        // endpoint path encodes none of these. Bounded like decodePathSegments; a real path needs 0-1 passes.
-        String decoded = rawEndpointPath;
-        for (int pass = 0; pass < 10; pass++) {
-            for (int i = 0, n = decoded.length(); i < n; i++) {
-                char c = decoded.charAt(i);
-                if (c == '\\') {
-                    return true;
-                }
-                if (c == '%' && i + 2 < n) {
-                    char a = decoded.charAt(i + 1);
-                    char b = decoded.charAt(i + 2);
-                    if ((a == '2' && (b == 'f' || b == 'F' || b == '5')) || (a == '5' && (b == 'c' || b == 'C'))) {
-                        return true;
-                    }
-                }
-            }
-            String next = percentDecodeOnce(decoded);
-            if (next.equals(decoded)) {
-                break;
+        // A real OIDC endpoint path is plain ASCII with no percent-encoding and no backslash, so reject either
+        // outright rather than try to out-decode the server. Percent-encoding is exactly where a tampered
+        // /settings hides a path separator ('/', '\') or a '..' that only surfaces once the server unescapes -
+        // and not only the forms this client's byte-oriented percentDecodeOnce resolves (%2f, %5c, %25, %252f,
+        // %2%66), but ones it deliberately does NOT: an overlong-UTF-8 %c0%ae or %e0%80%ae, or an IIS-style
+        // %u002e, which a permissive server decodes to '/' or '.' yet a single-byte decode leaves as high bytes
+        // or literal text - so they would sail past the segment scan in isEndpointUnderIssuerPath and, sitting
+        // past the issuer prefix, slip the scope. A literal backslash likewise folds to '/' on some proxies.
+        // Failing closed on any '%' or '\' keeps the issuer-path scope airtight against every encoding trick; a
+        // provider that genuinely percent-encodes its endpoint path must be configured explicitly with
+        // OidcDeviceAuth.builder(), which pins the origin only.
+        for (int i = 0, n = rawEndpointPath.length(); i < n; i++) {
+            char c = rawEndpointPath.charAt(i);
+            if (c == '%' || c == '\\') {
+                return true;
             }
-            decoded = next;
         }
         return false;
     }
@@ -1032,9 +1022,11 @@ private boolean adopt(PersistedToken token) {
         }
         // the file is attacker-writable, so treat the served token (the one getToken() puts verbatim into an
         // Authorization header or a PG-wire password) as untrusted: reject a control/non-ASCII char - and the
-        // whole entry - rather than route a tampered credential onto the wire. A null served token is unusable.
+        // whole entry - rather than route a tampered credential onto the wire. A null OR empty served token is
+        // unusable: an empty string passes hasOnlyTokenChars vacuously but would be served as a blank
+        // "Bearer " header that only draws a 401, so reject it here and fall through to a refresh or sign-in.
         String servedToken = groupsInToken ? token.getIdToken() : token.getAccessToken();
-        if (servedToken == null || !hasOnlyTokenChars(servedToken)) {
+        if (servedToken == null || servedToken.isEmpty() || !hasOnlyTokenChars(servedToken)) {
             return false;
         }
         accessToken = token.getAccessToken();
@@ -2016,8 +2008,16 @@ static Endpoint parse(String url) {
             int port;
             if (colon >= 0) {
                 host = hostPort.substring(0, colon);
+                String portStr = hostPort.substring(colon + 1);
+                // reject a leading '+': Integer.parseInt would read ":+443" as 443 and slip the range check,
+                // but a real authority port is bare digits. A leading '-' or any non-digit still flows to
+                // parseInt below, which rejects it (a negative fails the 1..65535 range check, a non-number
+                // throws NumberFormatException) - so only the '+' that parseInt silently accepts is caught here
+                if (portStr.isEmpty() || portStr.charAt(0) == '+') {
+                    throw new OidcAuthException().put("invalid url, could not parse the port [url=").put(url).put(']');
+                }
                 try {
-                    port = Integer.parseInt(hostPort.substring(colon + 1));
+                    port = Integer.parseInt(portStr);
                 } catch (NumberFormatException e) {
                     throw new OidcAuthException().put("invalid url, could not parse the port [url=").put(url).put(']');
                 }
@@ -2038,9 +2038,16 @@ static Endpoint parse(String url) {
             // advertised by a tampered /settings could otherwise pass the pin against the issuer. LDH ASCII
             // hosts, punycode (xn--...) and dotted IPv4 are all ASCII and unaffected.
             for (int i = 0, n = host.length(); i < n; i++) {
-                if (host.charAt(i) > 0x7f) {
+                char hc = host.charAt(i);
+                if (hc > 0x7f) {
                     throw new OidcAuthException().put("invalid url, the host contains a non-ASCII character [url=").put(url).put(']');
                 }
+                // reject a backslash in the host: the WHATWG URL spec folds '\' to '/', so a host like
+                // good.com\.evil.com could be re-split by a lenient consumer into a different authority. The OS
+                // resolver this client hands the host to never resolves such a name anyway, so fail closed.
+                if (hc == '\\') {
+                    throw new OidcAuthException().put("invalid url, the host contains a backslash [url=").put(url).put(']');
+                }
             }
             return new Endpoint(host, port, path, isTls);
         }

core/src/main/java/io/questdb/client/cutlass/json/JsonLexer.java

@@ -416,9 +416,10 @@ private CharSequence unescape(CharSequence raw) {
                     }
                     break;
                 default:
-                    // unknown escape: keep the backslash and the escaped character verbatim (lenient), so a
-                    // literal backslash in non-conformant input (e.g. a Windows path in an error body) is not
-                    // dropped
+                    // an unrecognized escape letter: keep the backslash and the char verbatim (lenient), so a
+                    // stray '\' before a non-escape char in non-conformant input survives rather than being
+                    // dropped. A '\' before a RECOGNIZED escape letter (" \ / b f n r t u) is still decoded by
+                    // the cases above - standard JSON unescape - so only genuinely unknown sequences reach here.
                     unescapeSink.put('\\').put(esc);
                     i += 2;
                     break;

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

@@ -595,6 +595,33 @@ public void testStoreLoadedAtMostOncePerInstance() throws Exception {
         });
     }
 
+    @Test(timeout = 30_000)
+    public void testTamperedEmptyServedTokenRejectedOnLoad() throws Exception {
+        assertMemoryLeak(() -> {
+            AtomicInteger device = new AtomicInteger();
+            MockOidcServer.Handler handler = (method, path, body) -> {
+                if (DEVICE_PATH.equals(path)) {
+                    device.incrementAndGet();
+                    return MockOidcServer.json(200, deviceAuthJson());
+                }
+                return MockOidcServer.json(200, tokenJson("ACCESS-FRESH", null, "REFRESH-1", 3600));
+            };
+            try (MockOidcServer server = new MockOidcServer(handler)) {
+                FakeTokenStore fake = new FakeTokenStore();
+                // a tampered persisted entry with an EMPTY served token passes hasOnlyTokenChars vacuously but
+                // would be served as a blank "Bearer " header; adopt() must reject it (not serve "") and fall
+                // back to the device flow, exactly like a control-char token
+                fake.loadReturns = new PersistedToken("", null, "REFRESH-1", System.currentTimeMillis() + 300_000, 300_000);
+                try (OidcDeviceAuth auth = baseBuilder(server).tokenStore(fake).build()) {
+                    String result = auth.signIn();
+                    Assert.assertEquals("ACCESS-FRESH", result);
+                    Assert.assertNotEquals("", result);
+                }
+                Assert.assertTrue("a rejected empty persisted token must fall back to the device flow", device.get() >= 1);
+            }
+        });
+    }
+
     @Test(timeout = 30_000)
     public void testTamperedFarFutureExpiryIsBoundedNotTrustedForever() throws Exception {
         assertMemoryLeak(() -> {

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

@@ -1075,6 +1075,9 @@ public void testEndpointParseRejectsMalformedUrls() {
         assertBuildFails("https://idp:0/d", "https://idp/t", "between 1 and 65535");
         assertBuildFails("https://idp:-1/d", "https://idp/t", "between 1 and 65535");
         assertBuildFails("https://idp/d", "https://idp:70000/t", "between 1 and 65535");
+        // a leading '+' on the port is rejected: Integer.parseInt would read ":+443" as 443, but a real
+        // authority port is bare digits (a leading '-' is already caught by the range check above)
+        assertBuildFails("https://idp:+443/d", "https://idp/t", "could not parse the port");
         // a host carrying control characters or whitespace (e.g. a smuggled CR/LF that would inject into the
         // outbound Host header) is rejected rather than passed verbatim to the transport
         assertBuildFails("https://ho\r\nst/d", "https://idp/t", "illegal character");
@@ -1105,6 +1108,9 @@ public void testEndpointParseRejectsMalformedUrls() {
         // U+212A -> k, ...), so a homoglyph host could otherwise pass the origin pin against a pinned issuer
         assertBuildFails("https://\u0130dp/d", "https://idp/t", "non-ASCII"); // U+0130, folds to i
         assertBuildFails("https://idp/d", "https://\u212Aelvin/t", "non-ASCII"); // U+212A Kelvin, folds to k
+        // a backslash in the host is rejected: the WHATWG URL spec folds '\' to '/', so a lenient consumer
+        // could re-split good.com\.evil.com into a different authority
+        assertBuildFails("https://good.com\\.evil.com/d", "https://idp/t", "backslash");
     }
 
     @Test(timeout = 30_000)
@@ -1934,6 +1940,12 @@ public void testIssuerPathScopingRejectsSplitEncodedAndBackslashSeparators() thr
         // rejects it at the encoded level too, before decodePathSegments would fold it to '/'
         Assert.assertFalse(invokeIsEndpointUnderIssuerPath("https://idp.example.com/realms/acme%5cevil/token", issuer));
         Assert.assertFalse(invokeIsEndpointUnderIssuerPath("https://idp.example.com/realms/acme%5Cevil/token", issuer));
+        // overlong-UTF-8 (%c0%ae, %e0%80%ae) and an IIS-style %u002e encode a '.'/'/' that a permissive server
+        // resolves but a byte-oriented percent decode leaves as high bytes or literal text; sitting past the
+        // issuer prefix they would slip the '..'/segment scan, so any '%' in an endpoint path is rejected
+        Assert.assertFalse(invokeIsEndpointUnderIssuerPath("https://idp.example.com/realms/acme/%c0%ae%c0%ae/evil/token", issuer));
+        Assert.assertFalse(invokeIsEndpointUnderIssuerPath("https://idp.example.com/realms/acme/%e0%80%ae%e0%80%ae/evil/token", issuer));
+        Assert.assertFalse(invokeIsEndpointUnderIssuerPath("https://idp.example.com/realms/acme/%u002e%u002e/evil/token", issuer));
     }
 
     @Test(timeout = 30_000)