Skip to content

fix: avoid panic when truncating multi-byte DB error messages - #272

Open
MoneyBund wants to merge 1 commit into
tempoxyz:mainfrom
MoneyBund:fix/sanitize-error-panic
Open

fix: avoid panic when truncating multi-byte DB error messages#272
MoneyBund wants to merge 1 commit into
tempoxyz:mainfrom
MoneyBund:fix/sanitize-error-panic

Conversation

@MoneyBund

Copy link
Copy Markdown

fix: avoid panic when truncating multi-byte DB error messages

Summary

sanitize_db_error truncated over-long messages with &error[..500], a byte
slice. When byte 500 lands in the middle of a multi-byte UTF-8 codepoint, that
slice panics with byte index 500 is not a char boundary.

The body of a PostgreSQL error is attacker-controlled: Postgres echoes the
offending literal back verbatim (invalid input syntax for type bigint: "…").
A caller can therefore craft a /query request whose resulting error message is
just over 500 bytes with a 2- or 4-byte character straddling the cut point, and
the request-handling task panics.

The router mounts only CorsLayer + TraceLayer (no CatchPanicLayer) and the
crate builds with the default panic = "unwind", so the panic unwinds the
connection task and the client's connection is reset with no HTTP response —
a trivially scriptable availability nuisance.

Reproduction (before)

GET /query?chainId=1&sql=SELECT * FROM blocks WHERE num = '<~497 ASCII bytes then "é">'

Postgres returns a >500-byte error; the slice lands mid-codepoint → panic →
dropped connection instead of a clean 422.

Fix

Truncate on a character boundary using char_indices(), and cap by character
count rather than byte length:

let error = match error.char_indices().nth(MAX_DB_ERROR_CHARS) {
    Some((boundary, _)) => format!("{}...", &error[..boundary]),
    None => error.to_string(),
};

char_indices().nth(n) yields the byte offset of the n-th character, which is
always a valid boundary, so the slice can never split a codepoint. The retained
prefix is unchanged for the common all-ASCII case.

Tests

Added test_sanitize_truncates_multibyte_without_panic, which sweeps the pad
length across the boundary (495..505) with 2-byte é and a 4-byte emoji. Without
the fix these inputs panic. Existing sanitizer tests still pass (cargo test --lib sanitize).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant