[fix](audit) escape 0x1F/0x1E in audit_log stream load to prevent row forgery - #66580
Open
CalvinKirs wants to merge 1 commit into
Open
[fix](audit) escape 0x1F/0x1E in audit_log stream load to prevent row forgery#66580CalvinKirs wants to merge 1 commit into
CalvinKirs wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
… forgery The audit plugin frames its stream-load payload for __internal_schema.audit_log with 0x1F (column separator) and 0x1E (row delimiter), but the string columns in AuditLoader.fillLogBuffer were appended without escaping. A statement carrying raw 0x1F/0x1E bytes -- e.g. inside a block comment or string literal, which the lexer accepts -- could therefore close its own audit row early and have the trailing bytes parsed as an additional, fully attacker-controlled row, forging or misattributing rows in the audit table (CWE-117 log injection). - Add sanitizeField(), 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 appendField() / appendLastField() so none can bypass the sanitizer and new string columns are covered automatically. Note that planTimesMs, getMetaTimesMs and scheduleTimesMs are String columns despite the Ms suffix. Numeric and boolean columns are appended directly since they can never contain these bytes. - Add unit tests asserting that injected delimiters (in stmt, user, db and planTimesMs) 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.
CalvinKirs
force-pushed
the
fix-o07-audit-log-injection
branch
from
August 7, 2026 09:10
bed1d7c to
90bba2a
Compare
gavinchou
approved these changes
Aug 7, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
approved
Contributor
|
PR approved by anyone and no changes requested. |
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29375 ms |
Contributor
TPC-DS: Total hot run time: 158344 ms |
Contributor
ClickBench: Total hot run time: 23.72 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
The builtin audit plugin frames its stream-load payload for
__internal_schema.audit_logwith0x1Fas the column separator and0x1Eas the row delimiter (seeAuditLoader.AUDIT_TABLE_COL_SEPARATOR/
AUDIT_TABLE_LINE_DELIMITER). InAuditLoader.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/0x1Ecould end its own audit row early and havethe 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
sanitizeField()inAuditLoader, which replaces the two framingbytes (
0x1F,0x1E) with a space. Only these two bytes arestructural, so all other content — including newlines and tabs already
present in SQL text — is preserved unchanged.
fillLogBufferthrough the newappendField()helper so that new string columns added in the futureare covered automatically. Numeric and boolean columns are appended
directly since they can never contain these bytes.
columns, and that ordinary statements pass through unchanged.
The text-file audit sink (
AuditLogBuilder,fe.audit.log) uses a|key=valueformat and is unaffected.Types of changes
Further comments
Behavior-preserving: only the two structural bytes, which are not
meaningful data, are affected. Existing clusters and audit consumers are
unchanged.