Skip to content

Commit a251a9a

Browse files
Sanitize peer / leader addresses in error messages; cap handshake-failure body
Three security-adjacent client error-text fixes: - ``DqliteProtocol._addr_suffix()`` interpolated ``self._address`` directly into exception messages. The address can be server- controlled in the leader-redirect reconnect path (LeaderResponse.address is intentionally not sanitised at decode — sanitising would split allowlist sets) so a hostile leader could inject CRLF / control-chars / U+2028 / U+2029 into the address and produce log lines that splice across rows. Sanitise via ``_sanitize_server_text`` before interpolation. - ``cluster._find_leader_impl``'s aggregate error builder interpolated ``node.address`` directly at three sites (no-leader-known, timed-out, transport-failure). Same hostile- leader risk on the redirect-reconnect path. Sanitise each interpolation. - ``DqliteProtocol.handshake``'s failure-response raise wrapped ``_failure_text(response)`` directly in a bare ProtocolError, bypassing the OperationalError display-cap that bounds every other raise site in the protocol module. Run the body through ``_truncate_error`` so a hostile-or-buggy peer cannot inflate the handshake error into a multi-KiB log line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 24da777 commit a251a9a

2 files changed

Lines changed: 37 additions & 5 deletions

File tree

src/dqliteclient/cluster.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,16 @@ async def _find_leader_impl(self, *, trust_server_heartbeat: bool) -> str:
351351
idx + 1,
352352
total_nodes,
353353
)
354-
errors.append(f"{node.address}: no leader known")
354+
# Sanitise the address before interpolating into the
355+
# aggregate error so a hostile leader cannot inject
356+
# CRLF / control-chars / line separators into log
357+
# output. ``node.address`` flows through
358+
# ``LeaderResponse.address`` which is deliberately NOT
359+
# sanitised at wire decode (allowlist semantics) —
360+
# sanitisation must happen at the user-facing error
361+
# boundary.
362+
_safe_addr = _sanitize_display_text(node.address)
363+
errors.append(f"{_safe_addr}: no leader known")
355364
continue
356365
except TimeoutError as e:
357366
logger.debug(
@@ -361,7 +370,7 @@ async def _find_leader_impl(self, *, trust_server_heartbeat: bool) -> str:
361370
idx + 1,
362371
total_nodes,
363372
)
364-
errors.append(f"{node.address}: timed out")
373+
errors.append(f"{_sanitize_display_text(node.address)}: timed out")
365374
last_exc = e
366375
continue
367376
except (DqliteConnectionError, ProtocolError, OperationalError, OSError) as e:
@@ -376,7 +385,7 @@ async def _find_leader_impl(self, *, trust_server_heartbeat: bool) -> str:
376385
idx + 1,
377386
total_nodes,
378387
)
379-
errors.append(f"{node.address}: {_truncate_error(str(e))}")
388+
errors.append(f"{_sanitize_display_text(node.address)}: {_truncate_error(str(e))}")
380389
last_exc = e
381390
continue
382391

src/dqliteclient/protocol.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,17 @@ async def handshake(self, client_id: int | None = None) -> int:
233233
# server-reported code and peer address so log aggregators
234234
# can group on the numeric code (DQLITE_PARSE,
235235
# DQLITE_NOTLEADER, etc.) rather than on text alone.
236+
# Truncate the failure body so a hostile-or-buggy peer
237+
# cannot inflate the handshake error into a multi-KiB
238+
# log line — the rest of the protocol module's
239+
# OperationalError raise sites get this for free via
240+
# ``OperationalError._MAX_DISPLAY_MESSAGE``; the raw
241+
# ProtocolError shape doesn't.
242+
from dqliteclient.cluster import _truncate_error
243+
236244
raise ProtocolError(
237-
f"Handshake failed: [{response.code}] {self._failure_text(response)}"
245+
f"Handshake failed: [{response.code}] "
246+
f"{_truncate_error(self._failure_text(response))}"
238247
)
239248

240249
if not isinstance(response, WelcomeResponse):
@@ -699,8 +708,22 @@ def _addr_suffix(self) -> str:
699708
700709
Returns an empty string when the address is unknown — keeping
701710
error messages clean for callers that don't thread it in.
711+
712+
Sanitises the address through ``_sanitize_server_text`` to
713+
strip control characters / CR / LF / U+2028 / U+2029 before
714+
interpolating it into an exception message. The address can
715+
be server-controlled in the leader-redirect reconnect path
716+
(LeaderResponse.address is intentionally NOT sanitised at
717+
decode time — sanitising would split allowlist sets — so the
718+
user-facing error/log boundary must do it instead). Without
719+
this, a hostile leader could inject CRLF into the address
720+
and produce log lines that splice across rows.
702721
"""
703-
return f" to {self._address}" if self._address else ""
722+
if not self._address:
723+
return ""
724+
from dqlitewire.messages.responses import _sanitize_server_text
725+
726+
return f" to {_sanitize_server_text(self._address)}"
704727

705728
def _failure_text(self, response: FailureResponse) -> str:
706729
"""Render a FailureResponse as the body string for an

0 commit comments

Comments
 (0)