fix: stop sending python tracebacks to remote dbus callers#659
Merged
Conversation
Two error paths embedded traceback.format_exc() / format_tb() in the error message body sent back to the caller: - message_bus.py:837 — INTERNAL_ERROR for a raising add_message_handler callback (the path issue #645 calls out). - send_reply.py:45 — SERVICE_ERROR for a raising exported service method (the sibling leak; same class of bug). The traceback discloses absolute install paths (revealing the install location and user-account names like /home/<user>/.../site-packages/...), exact source line numbers, function names, locals referenced in frames, and library versions to any peer that can invoke a method on the service. Useful for an attacker chaining further exploits; unnecessary for legitimate callers who just need to know the call failed. Sanitize both wire bodies to "<prefix>: <ExceptionClass>" and log the full traceback locally via _LOGGER.exception so operators retain diagnostics. send_reply.py gains its own logger since it had none. closes #645
…ation #636 added test_send_reply_generic_exception with `assert "boom" in body` as an incidental coverage assertion for the SERVICE_ERROR branch. After the traceback-leak fix the wire body no longer carries the exception's str(), only the class name, so the old assertion fails. Update it to match — pin that "boom" is NOT in the body and "RuntimeError" IS, mirroring the live-bus test added alongside the production change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #659 +/- ##
==========================================
+ Coverage 89.31% 89.55% +0.23%
==========================================
Files 29 29
Lines 3511 3512 +1
Branches 602 602
==========================================
+ Hits 3136 3145 +9
+ Misses 226 218 -8
Partials 149 149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates an information-disclosure issue in dbus-fast by ensuring error replies sent over D-Bus no longer embed Python tracebacks, while still preserving operator diagnostics via local logging.
Changes:
- Sanitize
INTERNAL_ERRORreplies produced whenadd_message_handlercallbacks raise, returning only the exception class name. - Sanitize
SERVICE_ERRORreplies produced when exported service-interface methods raise, returning only the exception class name and logging the full traceback locally. - Extend/add tests to assert that reply bodies do not contain tracebacks, file paths, or exception messages.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/dbus_fast/message_bus.py | Removes traceback leakage from INTERNAL_ERROR replies for raising user message handlers. |
| src/dbus_fast/send_reply.py | Logs full tracebacks locally and sanitizes SERVICE_ERROR reply bodies to exception class name only. |
| tests/test_send_reply.py | Updates assertions to ensure SERVICE_ERROR reply bodies are sanitized. |
| tests/service/test_methods.py | Strengthens service-method tests to verify sanitized error bodies for unexpected exceptions. |
| tests/test_aio_low_level.py | Adds a low-level regression test covering the INTERNAL_ERROR path for raising message handlers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
What
Sanitize the two error-response bodies that were embedding
traceback.format_exc()/traceback.format_tb()verbatim in messages sent back to remote callers. The wire body now contains only the exception class name; the full traceback is logged locally via_LOGGER.exceptionfor operator diagnostics.Why
A method handler that raised an unexpected exception sent back a body like:
That discloses absolute filesystem paths (revealing the install location and user-account names like
/home/<user>/.../site-packages/...), exact source line numbers, function names, locals referenced in frames, and library versions to any peer that can invoke a method on the service. On the system bus this means any unprivileged process. The information is useful for an attacker chaining further exploits (path-traversal targeting, version fingerprinting, internal class structure for crafted payloads) and unnecessary for legitimate callers, who only need to know the call failed.Two sites were leaking:
src/dbus_fast/message_bus.py:837—INTERNAL_ERRORreply for a raisingadd_message_handlercallback. This is the path Security: Full Python tracebacks sent to remote callers leak file paths and code structure #645 calls out.src/dbus_fast/send_reply.py:45—SERVICE_ERRORreply for a raising exported service-interface method. Sibling leak the issue did not flag; same class of bug, same fix.How
Both bodies become
"<prefix>: <ExceptionClass>". The full traceback is logged via_LOGGER.exceptionon the bus side so operators retain diagnostics.send_reply.pygains its own module logger since it had none.import tracebackremoved frommessage_bus.py(no longer needed);send_reply.pydropstracebackin favour oflogging.Tests
tests/service/test_methods.py::test_methods— extended the existingthrows_unexpected_errorassertion to pin the sanitizedSERVICE_ERRORbody (noTraceback, noFile ", nostr(e)content; class nameExceptionpresent).tests/test_aio_low_level.py::test_internal_error_reply_does_not_leak_traceback— new test for theINTERNAL_ERRORpath: registers anadd_message_handlercallback that raisesValueError("must not appear on the wire"), sends a call, asserts the reply body is sanitized (no traceback, no path, no exception message; class nameValueErrorpresent).closes #645