Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -149,22 +149,22 @@ private void fillLogBuffer(AuditEvent event, StringBuilder logBuffer) {
// 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);
Expand All @@ -183,40 +183,80 @@ private void fillLogBuffer(AuditEvent event, StringBuilder logBuffer) {

// 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,67 @@ private String getAuditLogBuffer(AuditLoader auditLoader) {
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;
}
}
Loading