Skip to content

Commit d5bcf93

Browse files
glasstigerclaude
andcommitted
Escape control/bidi chars in flush error messages
throwOnHttpErrorResponse echoed the raw HTTP error body into the LineSenderException message on three paths - the 401/403 auth body, a non-JSON body, and the toException JSON-parse-failure fallback - while only the parsed-JSON path went through putAsPrintable. A hostile, MITM'd or proxied endpoint could splice ANSI escapes, CR/LF or bidi overrides into a logged or printed exception (terminal hijack, log forging, visual spoofing). Route all three paths through LineSenderException.putAsPrintable so the server body is escaped just like the parsed-JSON fields already are. Add LineHttpSenderErrorResponseTest cases for the auth, non-JSON and malformed-JSON paths, each asserting a smuggled ESC or bidi override surfaces escaped, never raw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 67c78d9 commit d5bcf93

2 files changed

Lines changed: 132 additions & 12 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/line/http/AbstractLineHttpSender.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,9 @@ private void throwOnHttpErrorResponse(DirectUtf8Sequence statusCode, HttpClient.
837837
chunkedResponseToSink(response, sink);
838838
LineSenderException ex = new LineSenderException("Could not flush buffer: HTTP endpoint authentication error", retryable);
839839
if (sink.length() > 0) {
840-
ex = ex.put(": ").put(sink);
840+
// sanitize the raw server body before it reaches the exception message (and any log/terminal):
841+
// an untrusted or proxied endpoint must not splice control, ANSI or bidi chars into the render
842+
ex = ex.put(": ").putAsPrintable(sink);
841843
}
842844
ex.put(" [http-status=").put(statusAscii).put(']');
843845
client.disconnect();
@@ -855,11 +857,14 @@ private void throwOnHttpErrorResponse(DirectUtf8Sequence statusCode, HttpClient.
855857
}
856858
// ok, no JSON, let's do something more generic
857859
sink.clear();
858-
sink.put("Could not flush buffer: ");
859860
chunkedResponseToSink(response, sink);
860-
sink.put(" [http-status=").put(statusCode).put(']');
861+
// sanitize the raw server body before it reaches the exception message (and any log/terminal):
862+
// an untrusted or proxied endpoint must not splice control, ANSI or bidi chars into the render
863+
LineSenderException ex = new LineSenderException("Could not flush buffer: ", retryable)
864+
.putAsPrintable(sink)
865+
.put(" [http-status=").put(statusCode.asAsciiCharSequence()).put(']');
861866
client.disconnect();
862-
throw new LineSenderException(sink, retryable);
867+
throw ex;
863868
}
864869

865870
private void validateNotClosed() {
@@ -1067,7 +1072,9 @@ LineSenderException toException(Response chunkedRsp, DirectUtf8Sequence httpStat
10671072
while ((fragment = chunkedRsp.recv()) != null) {
10681073
jsonSink.putNonAscii(fragment.lo(), fragment.hi());
10691074
}
1070-
exception.put(jsonSink).put(" [http-status=").put(httpStatus.asAsciiCharSequence()).put(']');
1075+
// sanitize the raw server body before it reaches the exception message (and any log/terminal):
1076+
// an untrusted or proxied endpoint must not splice control, ANSI or bidi chars into the render
1077+
exception.putAsPrintable(jsonSink).put(" [http-status=").put(httpStatus.asAsciiCharSequence()).put(']');
10711078
reset();
10721079
return exception;
10731080
}

core/src/test/java/io/questdb/client/test/cutlass/line/LineHttpSenderErrorResponseTest.java

Lines changed: 120 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,56 @@
3333
import static io.questdb.client.test.tools.TestUtils.assertMemoryLeak;
3434

3535
/**
36-
* Verifies that the JSON error body a QuestDB HTTP endpoint returns on a failed flush is rendered
37-
* safely into the {@link LineSenderException} message. The JSON lexer resolves string escapes, so a
38-
* {@code message} or {@code errorId} field arrives fully decoded; a hostile or proxied endpoint could
39-
* otherwise smuggle real control characters or ANSI escapes that forge a log line or rewrite a
40-
* terminal when the exception text is printed. The sender must escape them, just as it does for column
41-
* names in an error message.
36+
* Verifies that the error body a QuestDB HTTP endpoint returns on a failed flush is rendered safely
37+
* into the {@link LineSenderException} message. A JSON error body has its string escapes resolved by the
38+
* lexer, so a {@code message} or {@code errorId} field arrives fully decoded; an auth (401/403) body, a
39+
* non-JSON body, and a body that fails to parse as JSON are echoed verbatim. In every case a hostile or
40+
* proxied endpoint could otherwise smuggle real control characters, ANSI escapes or bidi overrides that
41+
* forge a log line or rewrite a terminal when the exception text is printed. The sender must escape them,
42+
* just as it does for column names in an error message.
43+
* <p>
44+
* The dangerous bytes are built at runtime via {@code (char) 0x1b} (ESC) and {@code (char) 0x202e} (a
45+
* right-to-left override), so this source file stays pure ASCII and carries none of the chars it guards.
4246
*/
4347
public class LineHttpSenderErrorResponseTest {
4448

49+
// ESC: the lead byte of an ANSI escape sequence (terminal hijack)
50+
private static final char ESC = 0x1b;
51+
// U+202E RIGHT-TO-LEFT OVERRIDE: reorders displayed text (visual spoofing)
52+
private static final char RLO = 0x202e;
53+
54+
@Test(timeout = 30_000)
55+
public void testServerAuthErrorBodyControlAndBidiAreEscaped() throws Exception {
56+
assertMemoryLeak(() -> {
57+
// a 401/403 body is echoed into the exception verbatim (read as raw bytes, not through the JSON
58+
// parser), so a hostile or proxied endpoint could splice raw control, ANSI or bidi chars straight
59+
// into the LineSenderException; the sender must escape them just like the JSON-field path
60+
String errorBody = "denied " + ESC + "[2J forged\n" + RLO + "moc.live";
61+
try (MockOidcServer server = new MockOidcServer((method, path, body) -> MockOidcServer.chunkedJson(401, errorBody))) {
62+
try (Sender sender = Sender.builder(Sender.Transport.HTTP)
63+
.address("127.0.0.1:" + server.port())
64+
.protocolVersion(Sender.PROTOCOL_VERSION_V1)
65+
.disableAutoFlush()
66+
.build()) {
67+
sender.table("t").longColumn("v", 1L).atNow();
68+
try {
69+
sender.flush();
70+
Assert.fail("expected the server's auth error to surface as a LineSenderException");
71+
} catch (LineSenderException e) {
72+
String msg = e.getMessage();
73+
Assert.assertTrue(msg, msg.contains("authentication error"));
74+
Assert.assertTrue("visible text must be preserved: " + msg, msg.contains("denied"));
75+
Assert.assertTrue("the ESC must be escaped: " + msg, msg.contains("\\u001b"));
76+
Assert.assertTrue("the bidi override must be escaped: " + msg, msg.contains("\\u202e"));
77+
Assert.assertFalse("a raw ESC must not leak: " + msg, msg.indexOf(0x1b) >= 0);
78+
Assert.assertFalse("a raw newline must not leak: " + msg, msg.indexOf('\n') >= 0);
79+
Assert.assertFalse("a raw bidi override must not leak: " + msg, msg.indexOf(0x202e) >= 0);
80+
}
81+
}
82+
}
83+
});
84+
}
85+
4586
@Test(timeout = 30_000)
4687
public void testServerJsonErrorBidiAndZeroWidthAreEscaped() throws Exception {
4788
assertMemoryLeak(() -> {
@@ -117,11 +158,83 @@ public void testServerJsonErrorControlCharsAreEscaped() throws Exception {
117158
// ...but no raw control byte reaches the message: no ESC (ANSI injection) and no
118159
// newline (log-line forging); both arrive escaped instead
119160
Assert.assertTrue("the decoded ESC must be escaped, not raw: " + msg, msg.contains("\\u001b"));
120-
Assert.assertFalse("a raw ESC must not leak into the message: " + msg, msg.indexOf('\u001b') >= 0);
161+
Assert.assertFalse("a raw ESC must not leak into the message: " + msg, msg.indexOf(0x1b) >= 0);
121162
Assert.assertFalse("a raw newline must not leak into the message: " + msg, msg.indexOf('\n') >= 0);
122163
}
123164
}
124165
}
125166
});
126167
}
168+
169+
@Test(timeout = 30_000)
170+
public void testServerMalformedJsonErrorBodyControlAndBidiAreEscaped() throws Exception {
171+
assertMemoryLeak(() -> {
172+
// a body sent as application/json but not parseable as a QuestDB error object (a proxy/WAF page,
173+
// or an unexpected first key) makes the JSON parser throw; the fallback renders the raw body, which
174+
// must still be escaped. The unexpected first key "forged" forces the parse failure; the ESC and
175+
// bidi override ride in the value and must surface escaped, not raw
176+
String errorBody = "{\"forged\":\"x " + ESC + "[2J y " + RLO + " z\"}";
177+
try (MockOidcServer server = new MockOidcServer((method, path, body) -> MockOidcServer.chunkedJson(400, errorBody))) {
178+
try (Sender sender = Sender.builder(Sender.Transport.HTTP)
179+
.address("127.0.0.1:" + server.port())
180+
.protocolVersion(Sender.PROTOCOL_VERSION_V1)
181+
.disableAutoFlush()
182+
.build()) {
183+
sender.table("t").longColumn("v", 1L).atNow();
184+
try {
185+
sender.flush();
186+
Assert.fail("expected the malformed server response to surface as a LineSenderException");
187+
} catch (LineSenderException e) {
188+
String msg = e.getMessage();
189+
Assert.assertTrue(msg, msg.contains("Could not flush buffer"));
190+
// the raw body is shown (so the user can diagnose the unexpected response)...
191+
Assert.assertTrue("the raw body must be preserved: " + msg, msg.contains("forged"));
192+
// ...but the smuggled control and bidi chars arrive escaped, never raw
193+
Assert.assertTrue("the ESC must be escaped: " + msg, msg.contains("\\u001b"));
194+
Assert.assertTrue("the bidi override must be escaped: " + msg, msg.contains("\\u202e"));
195+
Assert.assertFalse("a raw ESC must not leak: " + msg, msg.indexOf(0x1b) >= 0);
196+
Assert.assertFalse("a raw bidi override must not leak: " + msg, msg.indexOf(0x202e) >= 0);
197+
}
198+
}
199+
}
200+
});
201+
}
202+
203+
@Test(timeout = 30_000)
204+
public void testServerNonJsonErrorBodyControlCharsAreEscaped() throws Exception {
205+
assertMemoryLeak(() -> {
206+
// a proxy or WAF can return a non-JSON error body (here text/plain) with raw ANSI/control bytes;
207+
// it reaches the generic error path, which must escape them before they hit a log or terminal.
208+
// The body is all ASCII (a real ESC and a newline) so it survives the raw response writer's
209+
// US-ASCII encoding; bidi is covered by the auth/malformed cases above
210+
String body = "upstream down " + ESC + "[31m forged\nsecond line";
211+
// hand-craft a chunked text/plain response: the generic path only reads the body when chunked, and
212+
// a non-application/json content type keeps it off the JSON parser
213+
String rawResponse = "HTTP/1.1 400 Bad Request\r\n"
214+
+ "Content-Type: text/plain\r\n"
215+
+ "Transfer-Encoding: chunked\r\n\r\n"
216+
+ Integer.toHexString(body.length()) + "\r\n" + body + "\r\n"
217+
+ "0\r\n\r\n";
218+
try (MockOidcServer server = new MockOidcServer((method, path, b) -> MockOidcServer.raw(rawResponse))) {
219+
try (Sender sender = Sender.builder(Sender.Transport.HTTP)
220+
.address("127.0.0.1:" + server.port())
221+
.protocolVersion(Sender.PROTOCOL_VERSION_V1)
222+
.disableAutoFlush()
223+
.build()) {
224+
sender.table("t").longColumn("v", 1L).atNow();
225+
try {
226+
sender.flush();
227+
Assert.fail("expected the server's non-JSON error to surface as a LineSenderException");
228+
} catch (LineSenderException e) {
229+
String msg = e.getMessage();
230+
Assert.assertTrue(msg, msg.contains("Could not flush buffer"));
231+
Assert.assertTrue("visible text must be preserved: " + msg, msg.contains("upstream down"));
232+
Assert.assertTrue("the ESC must be escaped: " + msg, msg.contains("\\u001b"));
233+
Assert.assertFalse("a raw ESC must not leak: " + msg, msg.indexOf(0x1b) >= 0);
234+
Assert.assertFalse("a raw newline must not leak: " + msg, msg.indexOf('\n') >= 0);
235+
}
236+
}
237+
}
238+
});
239+
}
127240
}

0 commit comments

Comments
 (0)