This is an automated email from the ASF dual-hosted git repository.
CalvinKirs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 4548f4b0596 [fix](audit) escape 0x1F/0x1E in audit_log stream load to
prevent row forgery (#66580)
4548f4b0596 is described below
commit 4548f4b0596021c3724935fbfe49d918a4e9111b
Author: Calvin Kirs <[email protected]>
AuthorDate: Mon Aug 10 09:38:46 2026 +0800
[fix](audit) escape 0x1F/0x1E in audit_log stream load to prevent row
forgery (#66580)
## Proposed changes
The builtin audit plugin frames its stream-load payload for
`__internal_schema.audit_log` with `0x1F` as the column separator and
`0x1E` as the row delimiter (see `AuditLoader.AUDIT_TABLE_COL_SEPARATOR`
/ `AUDIT_TABLE_LINE_DELIMITER`). In `AuditLoader.fillLogBuffer`,
however,
the string columns — statement text, catalog/db, user, changed
variables, error message, workload group, etc. — were appended without
escaping.
Because these fields can carry user-controlled content (and a SQL
statement may legitimately contain arbitrary bytes inside a block
comment or string literal, which the lexer accepts), a crafted statement
containing raw `0x1F`/`0x1E` could end its own audit row early and have
the trailing bytes parsed as an additional, fully attacker-controlled
row. This allows forging or misattributing rows in the audit table
(CWE-117 log injection).
### Changes
- Add `sanitizeField()` in `AuditLoader`, which replaces the two framing
bytes (`0x1F`, `0x1E`) with a space. Only these two bytes are
structural, so all other content — including newlines and tabs already
present in SQL text — is preserved unchanged.
- Route every string column in `fillLogBuffer` through the new
`appendField()` helper so that new string columns added in the future
are covered automatically. Numeric and boolean columns are appended
directly since they can never contain these bytes.
- Add unit tests asserting that injected delimiters cannot add rows or
columns, and that ordinary statements pass through unchanged.
The text-file audit sink (`AuditLogBuilder`, `fe.audit.log`) uses a
`|key=value` format and is unaffected.
## Types of changes
- [x] Bugfix (non-breaking change which fixes an issue)
## Further comments
Behavior-preserving: only the two structural bytes, which are not
meaningful data, are affected. Existing clusters and audit consumers are
unchanged.
---
.../org/apache/doris/plugin/audit/AuditLoader.java | 82 ++++++++++++++++------
.../apache/doris/plugin/audit/AuditLoaderTest.java | 63 +++++++++++++++++
2 files changed, 124 insertions(+), 21 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/plugin/audit/AuditLoader.java
b/fe/fe-core/src/main/java/org/apache/doris/plugin/audit/AuditLoader.java
index 6a09fdfa1fc..bbc86646635 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/plugin/audit/AuditLoader.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/plugin/audit/AuditLoader.java
@@ -149,22 +149,22 @@ public class AuditLoader extends Plugin implements
AuditPlugin {
// should be same order as InternalSchema.AUDIT_SCHEMA
// uuid and time
- logBuffer.append(event.queryId).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.queryId);
logBuffer.append(TimeUtils.longToTimeStringWithms(event.timestamp)).append(AUDIT_TABLE_COL_SEPARATOR);
// cs info
- logBuffer.append(event.clientIp).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.user).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.feIp).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.clientIp);
+ appendField(logBuffer, event.user);
+ appendField(logBuffer, event.feIp);
// default ctl and db
- logBuffer.append(event.ctl).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.db).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.ctl);
+ appendField(logBuffer, event.db);
// query state
- logBuffer.append(event.state).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.state);
logBuffer.append(event.errorCode).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.errorMessage).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.errorMessage);
// execution info
logBuffer.append(event.queryTime).append(AUDIT_TABLE_COL_SEPARATOR);
@@ -183,40 +183,80 @@ public class AuditLoader extends Plugin implements
AuditPlugin {
// plan info
logBuffer.append(event.parseTimeMs).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.planTimesMs).append(AUDIT_TABLE_COL_SEPARATOR);
-
logBuffer.append(event.getMetaTimesMs).append(AUDIT_TABLE_COL_SEPARATOR);
-
logBuffer.append(event.scheduleTimesMs).append(AUDIT_TABLE_COL_SEPARATOR);
+ // planTimesMs / getMetaTimesMs / scheduleTimesMs are String columns
(formatted timing
+ // breakdowns), not numbers, so they must be sanitized too.
+ appendField(logBuffer, event.planTimesMs);
+ appendField(logBuffer, event.getMetaTimesMs);
+ appendField(logBuffer, event.scheduleTimesMs);
logBuffer.append(event.hitSqlCache ? 1 :
0).append(AUDIT_TABLE_COL_SEPARATOR);
logBuffer.append(event.isHandledInFe ? 1 :
0).append(AUDIT_TABLE_COL_SEPARATOR);
// queried tables, views and m-views
-
logBuffer.append(event.queriedTablesAndViews).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.chosenMViews).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.queriedTablesAndViews);
+ appendField(logBuffer, event.chosenMViews);
// variable and configs
-
logBuffer.append(event.changedVariables).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.sqlMode).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.changedVariables);
+ appendField(logBuffer, event.sqlMode);
// type and digest
- logBuffer.append(event.stmtType).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.stmtType);
logBuffer.append(event.stmtId).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.sqlHash).append(AUDIT_TABLE_COL_SEPARATOR);
- logBuffer.append(event.sqlDigest).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.sqlHash);
+ appendField(logBuffer, event.sqlDigest);
logBuffer.append(event.isQuery ? 1 :
0).append(AUDIT_TABLE_COL_SEPARATOR);
logBuffer.append(event.isNereids ? 1 :
0).append(AUDIT_TABLE_COL_SEPARATOR);
logBuffer.append(event.isInternal ? 1 :
0).append(AUDIT_TABLE_COL_SEPARATOR);
// resource
-
logBuffer.append(event.workloadGroup).append(AUDIT_TABLE_COL_SEPARATOR);
-
logBuffer.append(event.cloudClusterName).append(AUDIT_TABLE_COL_SEPARATOR);
+ appendField(logBuffer, event.workloadGroup);
+ appendField(logBuffer, event.cloudClusterName);
// already trim the query in
org.apache.doris.qe.AuditLogHelper#logAuditLog
String stmt = event.stmt;
if (LOG.isDebugEnabled()) {
LOG.debug("receive audit event with stmt: {}", stmt);
}
- logBuffer.append(stmt).append(AUDIT_TABLE_LINE_DELIMITER);
+ // stmt is the last (and only free-text) column; sanitize it too so a
statement carrying
+ // raw 0x1F/0x1E cannot truncate its own row and forge a following one.
+ appendLastField(logBuffer, stmt);
+ }
+
+ /**
+ * Append one string column to the delimiter-framed audit stream-load
payload, followed by the
+ * column separator. The value is sanitized first so that user-controlled
text (SQL statement,
+ * identifiers, session-variable values, error messages, ...) cannot embed
the column separator
+ * (0x1F) or row delimiter (0x1E) and thereby forge, truncate, or
misattribute audit rows in the
+ * internal {@code audit_log} table (O07 / CWE-117 log injection). Numeric
and boolean columns
+ * are appended directly since they can never contain these bytes.
+ */
+ private static void appendField(StringBuilder logBuffer, String value) {
+
logBuffer.append(sanitizeField(value)).append(AUDIT_TABLE_COL_SEPARATOR);
+ }
+
+ /**
+ * Append the final string column of a row: sanitize the value (same
reason as {@link
+ * #appendField}) and terminate the row with the line delimiter. Every
string column is written
+ * through {@code appendField}/{@code appendLastField} so none can bypass
the sanitizer.
+ */
+ private static void appendLastField(StringBuilder logBuffer, String value)
{
+
logBuffer.append(sanitizeField(value)).append(AUDIT_TABLE_LINE_DELIMITER);
+ }
+
+ /**
+ * Replace the audit framing bytes (column separator 0x1F and row
delimiter 0x1E) with a space so
+ * field content cannot alter row/column framing. Only these two bytes are
structural, so other
+ * characters (including newlines and tabs already present in SQL text)
are preserved as-is.
+ */
+ private static String sanitizeField(String value) {
+ if (value == null || value.isEmpty()) {
+ return value;
+ }
+ if (value.indexOf(AUDIT_TABLE_COL_SEPARATOR) < 0 &&
value.indexOf(AUDIT_TABLE_LINE_DELIMITER) < 0) {
+ return value;
+ }
+ return value.replace(AUDIT_TABLE_COL_SEPARATOR, '
').replace(AUDIT_TABLE_LINE_DELIMITER, ' ');
}
// public for external call.
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/plugin/audit/AuditLoaderTest.java
b/fe/fe-core/src/test/java/org/apache/doris/plugin/audit/AuditLoaderTest.java
index 63bab9c3f95..2da3c3dfa1a 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/plugin/audit/AuditLoaderTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/plugin/audit/AuditLoaderTest.java
@@ -79,4 +79,67 @@ public class AuditLoaderTest {
StringBuilder buffer = Deencapsulation.getField(auditLoader,
"auditLogBuffer");
return buffer.toString();
}
+
+ // O07: raw 0x1F/0x1E in user-controlled fields must not be able to
add/remove columns or rows.
+ // A statement carrying the framing bytes (e.g. inside a block comment)
must still produce exactly
+ // one row with the same column count as a clean statement -- otherwise
the attacker forges a row.
+ @Test
+ public void testDelimiterInjectionDoesNotAlterFraming() {
+ AuditLoader auditLoader = new AuditLoader();
+ char col = AuditLoader.AUDIT_TABLE_COL_SEPARATOR;
+ char line = AuditLoader.AUDIT_TABLE_LINE_DELIMITER;
+
+ StringBuilder clean = new StringBuilder();
+ Deencapsulation.invoke(auditLoader, "fillLogBuffer",
+ new AuditEvent.AuditEventBuilder()
+ .setUser("alice").setDb("mydb").setStmt("select
1").build(),
+ clean);
+
+ // The forged payload tries to close its own row and inject a fully
attacker-controlled one.
+ // Inject into stmt, user, db AND planTimesMs -- planTimesMs is a
String column that is easy
+ // to overlook (its name suggests a number), so exercising it guards
against a column
+ // silently bypassing the sanitizer.
+ String evilStmt = "select 1 /*" + line + "deadbeef" + col +
"2026-01-01 00:00:00.000"
+ + col + "10.0.0.9" + col + "root" + col + "DROP TABLE
finance.ledger*/";
+ StringBuilder evil = new StringBuilder();
+ Deencapsulation.invoke(auditLoader, "fillLogBuffer",
+ new AuditEvent.AuditEventBuilder()
+ .setUser("al" + col + "ice").setDb("my" + line + "db")
+ .setPlanTimesMs("plan:" + col + "1ms" + line +
"forged")
+ .setStmt(evilStmt).build(),
+ evil);
+
+ // Exactly one row, and the same number of columns as the clean event.
+ Assert.assertEquals("injected 0x1E must not add rows",
+ count(clean, line), count(evil, line));
+ Assert.assertEquals("one row per event", 1, count(evil, line));
+ Assert.assertEquals("injected 0x1F must not add columns",
+ count(clean, col), count(evil, col));
+ // The forged tokens survive only as inert text, never as framing
bytes.
+ Assert.assertTrue(evil.toString().contains("DROP TABLE
finance.ledger"));
+ }
+
+ // The sanitizer must be a no-op for ordinary statements: no data loss, no
mutation.
+ @Test
+ public void testCleanStatementIsPreserved() {
+ AuditLoader auditLoader = new AuditLoader();
+ StringBuilder buffer = new StringBuilder();
+ Deencapsulation.invoke(auditLoader, "fillLogBuffer",
+ new AuditEvent.AuditEventBuilder()
+ .setUser("bob").setDb("sales")
+ .setStmt("select * from t where a = 1 and b =
'x'").build(),
+ buffer);
+ Assert.assertTrue(buffer.toString().contains("select * from t where a
= 1 and b = 'x'"));
+ Assert.assertEquals(1, count(buffer,
AuditLoader.AUDIT_TABLE_LINE_DELIMITER));
+ }
+
+ private static int count(CharSequence s, char c) {
+ int n = 0;
+ for (int i = 0; i < s.length(); i++) {
+ if (s.charAt(i) == c) {
+ n++;
+ }
+ }
+ return n;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
